• Little Bobby Tables (unregistered)

    Comment frist = new Comment (frist);

    Comment frist = new Comment (frist);

  • (nodebb)

    Doesn't the submitter know that if instantiating a class once is good, instantiating it twice is more gooder?

  • Mischa (unregistered)

    Where is the WTF? This is just a simple bug which probably is a result of a merge where two implementations got appended.

  • Gautam (unregistered)

    Always a warning sign when the developers say, this is always how its been.

  • (nodebb) in reply to Mischa

    Where is the WTF? This is just a simple bug which probably is a result of a merge where two implementations got appended.

    There's a possible WTF ^^^ right there, where you merge changes and don't look at the result. Definitely WTF-worthy.

  • Zach (unregistered)

    So, you're saying it's a coincidence that 400 tests passed? That's highly unlikely or they wrote really shitty tests. Come to think of it, they probably wrote really shitty tests

  • James (unregistered) in reply to Zach

    I bet they got intermittent failures on the unit tests in their CI builds, and no one knew why. Not that I'm speaking from experience or anything.

    (Yes, I'm assuming they had continuous integration. I really, really hope they did anyway.)

  • Shill (unregistered) in reply to Zach

    No, the server that was started was listening on a port in its own thread and would not have been garbage-collected. So the tests were fine. There was just a bug were an instance variable pointed to the wrong instance of a class.

  • Two Pi Man (unregistered)

    If there's no live reference to the instance then there's also nothing receiving incoming connections, so none of their tests can be connecting to it...

    Or they're doing something really dodgy in their finalize() method.

  • Zach (unregistered) in reply to Shill

    correct, but there is something WTF-y about the fact that none of their tests caught that

  • get off my lawn (unregistered)

    There must be something edited out of the story, it doesn't read complete.

  • sizer99 (google)

    ' It was much harder to understand how and why that line got there in the first place, or why no one had ever noticed it before.'

    The first is a head scratch (looks kind of like a copypasta oops), the second is easy - because their tests never failed nobody ever looked.

  • Temporarily banned from Stackoverflow (unregistered) in reply to Shill

    Exaclty, a Thread is a GC root, so there's no WTF, just a small WTH.

  • garbi (unregistered)

    Maybe they started without the second instance and it worked for the first test case. Then they added a second test case and the creation threw an exception like "port already in use", because the first instance was not shut down by the garbage collector before the second was created. So they deleted the reference to the first, and, by chance, the garbage collector kicked in just at the right time to make both test cases work. The WTF here seems to be letting the garbage collector choosing the time when the server is shut down instead of doing it explicitely. (And providing a static varibale in a parent class for use in multiple test cases, which creates depedencies between the test cases. Although it creates no problem in this special case.)

  • Live from production! (unregistered)

    I honestly expected this story to end by revealing that the "test instance" of the notification server was, in fact, the live production one and by attempting to restart the service, Alex had killed the demo product and cost the company hundreds of thousands of dollars in lost potential revenue.

  • (nodebb) in reply to Shill

    No, the server that was started was listening on a port in its own thread and would not have been garbage-collected.

    </<blockquote>> Shill is correct. Given that the component is referred to as a server, it would be natural to assume that there was some sort of "Active" component to it, be it a thread, or a callback registered with a web framework. So it's instance would not have gone out of scope, and therefore would never have been garbage collected.

    It's just a simple bug that the instance member was replaced with an uninitialised instance. Unfortunately, this shows us that nobody ever wrote tests to exercise the functionality of the server beyond the Init method. So, in essence, the shutdown behaviour was never tested.

    TRWTF is that whoever wrote the article doesn't know enough about how generational (or for that matter, any) garbage collection algorithms work in a multithreaded environment.

  • Fedaykin (unregistered)

    The real WTF is 'unit tests' that fire up a real instance and create other side effects.

  • Gumpy_Gus (unregistered)

    Reminds me of a major-league old-school cloud provider. They carefully wrote a backup program to backup all the user files every night. And a job scheduler to run the backup program every night at 2AM. And a report generator that sent a backup log to the line-printer to be delivered to the manager. All fine so far. The manager got a thick printout every morning of all files changed. Manager happy, everybody happy.

    Then about two months later a user wiped out their main file and called in to get the file restored.

    So the customer service folks, who were rather savvy, wrote the control cards to mount the backup tape and do an extract USER73/*.

    No soap. The write backup job mounted the tape as 6250BPI but actually wrote it at 1200BPI, which gives you a tape of garbage. Nobody had actually bothered to TEST the backup tapes, so all the backups were useless.

Leave a comment on “Collect Your Garbage Test”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article