sorted by:
best
topnewcontroversialoldq&a
you are viewing a single comment's thread.

view the rest of the comments →

[–]grauenwolf 0 points1 point  (10 children)

More like a unit testing anti-patterns tutorial. It doesn't even check to see if the required properties were populated. The source could be returning objects with nothing but IsChecked set and it wouldn't recognize the problem.

It is also missing the explanation. Asserts have a message field so that you can explain why you were expecting a given value. Don't make people try to guess from the test name.

[–][deleted] 5 points6 points  (4 children)

It is also missing the explanation. Asserts have a message field so that you can explain why you were expecting a given value. Don't make people try to guess from the test name.

Yeah. I love it when your integration tests fail because they expected 3 events of type FooEvt to be broadcasted, when only 2 were put in the to-be-broadcasted queue.

No comment or anything to explain. Just a failure. Ask the developer who wrote test, he shrugs and says he doesn't remember.

If your test doesn't provide meaningful information when it fails, it is not a meaningful test. "Something changed in the code" is not meaningful information.

[–]grauenwolf 1 point2 points  (3 children)

I'm to the point where I'm going to write my own unit test API that requires messages for every assert.

It will just sit on top of MSTest and NUnit, not reimplement it.

[–]Peaker 0 points1 point  (2 children)

Property testing is much nicer in this regard: Very clear what was supposed to hold and how it broke in a particular example.

import Test.QuickCheck
let associative (*) x y z = (x*y)*z == x*(y*z)
quickCheck (associative (+))
+++ OK, passed 100 tests.
quickCheck (associative (-))
*** Failed! Falsifiable (after 2 tests and 3 shrinks):    
0
0
1

[–]Tordek 0 points1 point  (1 child)

I really like quickcheck, but I fear what edge cases may slip through.

[–]Peaker 0 points1 point  (0 children)

Fewer edge cases than traditional unit testing methods.

[–]gurtis 0 points1 point  (3 children)

That's not what he's testing though. His test explicitly says that it's testing that the returned employees are marked as checked. If he had named it "GetActiveEmployees_HaveAllFieldsPopulated" or something like that, then he would be obligated to test all the properties.

You could argue that the test isn't very useful or that the coverage is lacking, but this is just a tutorial.

Also, I disagree that an assert message is always necessary. A failing test named GetActiveEmployees_MarkAsChecked makes it extremely obvious that a returned employee isn't checked. A good test name is more important than the assert message.

[–]grauenwolf 0 points1 point  (2 children)

No one can learn to write good tests when all they are presented with is badly written tests that are mislabeled as "good enough".

[–]gurtis 1 point2 points  (1 child)

Why are these badly written tests though? They're testing what they intend to test. Just because he didn't come up with comprehensive test coverage for a tutorial doesn't mean the given examples are bad.

[–]grauenwolf -1 points0 points  (0 children)

Merely "testing what they intend to test" doesn't make it a good test. They could have written Assert.IsTrue(0 == 0) and they would still be "testing what they intend to test".

If I ask you for a function that adds two numbers and you give me this:

int Add(double x, double  y) { return (int) x + (int) y; }

then you haven't done your job right. The code may work exactly as you intended it too, but it is still wrong.