• Tom (unregistered)

    How is that a WTF?

  • (nodebb)

    If the doctor finds that your health condition is not OK, he nonetheless tells you that you are OK.

  • Hal (unregistered) in reply to Tom

    Its a WTF because the implications are completely missed by the submitter. The value returned from internal_check_struct_integrity() is inverted and finally returned to the caller and it caused not tests to fail. Suggesting that the normal condition is whatever struct is being checked is usually failing the integrity check...

    Something is probably very very broken somewhere.

  • (nodebb)

    It's like Apple's "goto fail" with something related to SSL certs. The stakes in that case was more dangerous because it allowed some SSL checks to pass when it was supposed to fail. Also, if there's code standard in place, there's a chance that the standard has this company immune to such a mistake.

  • Robin (unregistered)

    Sure mistakes like this happen all the time, but that's one of the things unit tests are for. If something so easy to test as "this does something other than return OK if the integrity check fails" isn't being tested then I wonder how committed they actually are to testing.

  • David Hemming (unregistered)

    But it returns nothing in the other case? Isn't that going to cause problems?

  • seebs (unregistered)

    The code presumably keeps running and doing other things in the other case, this is a partial snippet, not the entire program.

  • I'm not a robot (unregistered) in reply to Hal
    The value returned from internal_check_struct_integrity() is inverted and finally returned to the caller and it caused not tests to fail. Suggesting that the normal condition is whatever struct is being checked is usually failing the integrity check...
    It's not inverted. If the integrity check fails then the outer function returns success to the caller, but if the check passes then it carries on as it's supposed to and as far as we know works perfectly. This simply means that they're not (properly) testing cases that fail the check.
  • I dunno LOL ¯\(°_o)/¯ (unregistered) 
        if (post != FRIST) {
        return FRIST;
    }

    It's better than waking up early enough in the morning!

  • Yikes (unregistered)

    The WTF is the set of tests. One of them should be passing in bad data and expected something other than OK back.

  • cellocgw (unregistered) in reply to David Hemming

    That was my thought too. Was this function ever called? And if so, what did the calling function expect? Could it handle an "empty" return value? Very weird.

  • Dave Aronson (unregistered)

    Two words for ya: Mutation Testing! That would have quickly revealed that their test site clearly sucks already, never mind adding any more uncovered code.

  • DrPepper (unregistered)

    It didn't cause any unit tests to fail

    So there must have been a test with the title "Returns OK when the integrity check fails"; and no code reviewer questioned that test title?

    I always read the test title; else how do you know what the test is supposed to be testing? How do you know you've covered all the cases? And how do you know that the test is actually testing what it intends to be testing?

    Anyone can write a set of tests that do not fail. It's much harder to write tests that actually test what needs to be tested.

  • DrPepper (unregistered)

    It didn't cause any unit tests to fail

    So there must have been a test with the title "Returns OK when the integrity check fails"; and no code reviewer questioned that test title?

    I always read the test title; else how do you know what the test is supposed to be testing? How do you know you've covered all the cases? And how do you know that the test is actually testing what it intends to be testing?

    Anyone can write a set of tests that do not fail. It's much harder to write tests that actually test what needs to be tested.

  • iWantToKeepAnon (unregistered)

    Make that guy a senior engineer or better yet a tech lead because he actually LOOKS at AND READs the code ... you know what happens when you ASSuME.

  • Ollie Jones (unregistered)

    I've made mistakes like this in my half-century as a programmer. Blush.

  • Wizofaus (unregistered)
    Comment held for moderation.
  • (nodebb)

    There is not enough context to tell if this is a WTF. The calling function might have a spec that says "do something to the struct but only if the struct passes integrity checks. Report any errors that occur during the stuff that you do. If you do nothing, report OK".

    Unlikely, but possible.

    Addendum 2022-06-09 06:52: Even if they did just return the wrong value, it's not really a WTF, it's just a bug that wasn't caught because of insufficient coverage in the unit tests.

  • Marvin the Martian (unregistered)

    It's like a mental health day for code: "It's OK to be not OK".

  • Neveranull (unregistered)

    It very well could be that the internal function’s output is reversed, and so it’s return value has to be reversed by the caller. That’s why I never name a function checkStatus(), but rather isStatusOK().

  • (nodebb)

    thanks for info

    Addendum 2022-10-30 20:31: I appreciate your fantastic program because I've been researching mental health for a while and need it. It will be harder to do in the future, so I advise everyone to become aware of their own shortcomings and spot any potential www.australiacounselling.com.au/ . In any event, if you want to feel better mentally, you can discover treatment close by and speak with a professional.

  • pnehl snap (unregistered)
    Comment held for moderation.

Leave a comment on “Mostly Okay”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article