45
Root
2y

expect([
row[‘blah’][0][1],
row[‘blah’][1][1],
row[’blah’][2][1],
row[‘blah’][3][1],
row[‘blah’][4][1],
]).to contain_exactly(
a.name(user), # “John doe”
c.name(user), # “John doe”
e.name(user), # “John doe”
b.name(user), # “John doe”
d.name(user), # “John doe”
)

(Note: The comments are mine.)

See the problem? No, not the ugly code (which is actually worse than what i posted here).

It’s using the same ridiculous getter (if you can call it that) that pulls a name out of the passed user object, and then expecting each row to have that name, in order. Not that order matters when they’re all the same.

Upon inspection, all objects created by the spec have the exact same name, so the above test passes (as long as there are 5 rows). It passes, but totally not because it should: those aren’t the objects that are actually in the table. All of the specs — all 22 of them — only check for that shared name on various rows, and no other data. And it’s not like this is the only issue, either.

Fuck me these are bad.

And this guy is a senior dev earning significantly more than me. Jesus what the fuck Christ.

Comments
  • 9
    Who doesn't like fake tests
  • 14
    @C0D4 🙋🏻‍♀️
  • 4
    Your stories read like a standup routine
  • 1
    Looks like a unit test stub. The template is there and could be copymodified to actually test the unit. But time ran out and instead of finishing the job, he abandoned it in favor of the next ticket.
  • 5
    "We pride ourselves in TDD and high code coverage" - that company, probably.
  • 2
    Besides the dumb test, this is the FIRST TIME I’ve seen an object passed to another object to get its own field. I have NEVER SEEN THIS BEFORE!! What happened to user.name()?
  • 3
    @TeachMeCode I know right!?
  • 4
    @Root damn. So in order to actually get the user name value you need to create a whole new object and pass the user to it??? And did he manually create the test data by creating new user objects and stuffing them in that array? It’s like he doesn’t even know what unit tests are for. Everytime I feel bad about my skill set I read stuff like this and feel hell lot better
  • 1
    @TeachMeCode These specs have been touched by all the “best” devs the company has to offer. So.

    They’re not *entirely* useless specs — they do test filtering. (Create 5, filter for x, expect to see 2.) but they don’t test anything else, or even that they’re the correct records. I suppose it also tests “doesn’t crash”?

    Yeah.
    They’re pretty bad, and I totally get what you mean. I look at the things I find, and I’m just amazed. Totally helped me break out of imposter syndrome and stay away from it.

    Also, idk about that getter. It does some pii decrypting and feature flag checks, but why isn’t it on user? It’s so odd.
  • 3
    On a previous company, a poor junior dev did not understand how the code was failing when executed, but all the test ok.

    Turns out the dev placed each case on try/catch blocks.

    The good news is that the Junior Dev understood what was wrong after a bit of a demonstration and removed the tey/catch blocks.

    The very bad news was a 28% of the test showed as Ok...

    So I can understand the frustration fixing this kind of stuff.
  • 1
    @Root wtf he could’ve done that in the user class. Decrypt the name after it’s pulled from the db and put it into the name field. Damn these guys are amazing at making spaghetti, in fact they should drop this whole software gig and apply to pizzerias!
  • 2
    > And this guy is a senior dev earning significantly more than me. Jesus what the fuck Christ.

    This is 100.000% BULLSHIT.

    This is also among the absolute worst pieces of test code I have ever seen, and I'm pretty much technically a junior. holy fuck. You should be making double what this asshole makes.
  • 1
    @Root

    > These specs have been touched by all the “best” devs the company has to offer. So.

    My faith in the industry has just fallen a bit. I am so sorry. This to me means that you are surely the most competent developer in your company.
  • 0
    @corscheid guy should be fired lol
  • 1
    @corscheid he’s probably still there bc his boss can’t judge good code from bad. Seriously if Root took over for her boss a huge percentage of the staff would be shitting their pants knowing they’re next on the guillotine list. There would be a mass apocalypse of firings left and right (which I would totally support bc from her posts a lot of people don’t do jack shit)
  • 3
    This is how he unit test? What in the actual fuck is this? @Root reply this to him on our behalf,

    expect (youGoFuckYourSelf(), true);
  • 3
    @kiki Long past time. But I want to find something else first, as I kinda still need the income to support my family.
Add Comment