6 ms·
What? People really would criticize that code because it has two assertions? How are they ever testing any state changes? And to the author: Your bubble is sig
by choeger 4y ago
What? People really would criticize that code because it has two assertions? How are they ever testing any state changes?
And to the author: Your bubble is significantly different from mine. Pretty much every competent developer I've worked with would laugh at you for the idea that the second test case would not be perfectly fine. (But that first iteration would never pass code review either because it does nothing and thus is a waste of effort.)
- mollerhoj 4y agoTo answer your question: We zealots test for the fact that something changes to some degree. E.g with rubys rspec library: expect { foo.call() }.to change { bar.value }.by(2) That is, regardless of the absolute value of bar.value, I expect foo.call() to increment it by 2. The point of the 1 assertion per test guideline is to end up with tests that are more focused. Giving that you did not seem to think of the above technique, I'd say that this guideline might just have helped you discover a way to write better specs ;-) Guidelines (that is, not rules) are of course allowed to be broken if you have a good reason to do so. But not knowing about common idioms is not a good reason. You might argue that the above code is just sugar for 2 assertions, but thats beside the point: The test is more focused, there -appears- to be only one assertion, and thats what matters.
- philliphaydon 4y agoThat’s because the example test only requires 1 assertion. Any rule that says there should be only 1 assertion ever is stupid.
- mollerhoj 4y agoOP asked how any state change would be tested with a single 'assertion' and I provided an answer. Absolute rules are stupid, but our codebase has just short of 10k tests, and very few have more than one assertion. The only reason I can really see to have more than one assertion would be to avoid having to run the setup/teardown multiple times. However, its usually a desirable goal to write code that require little setup/teardown to test anyways because that comes with other benefits. Again, it might not be practical or even possible, but that goes of almost all programming "rules"..
- dd82 4y agoone assert per test seems... as you said, indicative of zealotry. if you already have your object there, why not test for the changes you expect? So you have one test that indicates that a log error is outut. then another that tests that the property X in the return from the error is what you expect. then another test to determine that propery Y in return is what you expect? that to me is wasteful, unclear, bloated. About the only useful result I can see that is it allows bragging about how many tests a project has.
- mannykannot 4y agoFurthermore, if you have a one-assertion rule, some bright spark will realize he can write a single assertion that checks for the conjunction of all the individual postconditions. That's one way to get dogma-driven assertion roulette, as you will not know which particular error occurred.
- seadan83 4y agoIf all assertions are at the end of the test, then yes. Sometimes this can be made nice with custom matchers, eg: assertThat(fooReturningOptional(), isPresentAndIs(4)) Or assertThat(shape, hasAreaEqualTo(10)) Or AssertThat(polygonList, hasNoIntersections()) Custom matchers can go off the deep end really easily. One of those cases of learn the principle, then learn when it does not apply
- jeremyjh 4y agoIf two tests call the same method with the same setup and arguments just to assert two different outcomes I would suggest that is the code smell.
- dd82 4y agoYep, but that seems to be the prevailing convention being asserted (pun intended) by many commenters.
- philliphaydon 4y ago
- krona 4y agoYou tested a postcondition. What about preconditions and invariants, do you have separate tests for those assertions too, or just not bother?
- mollerhoj 4y agoPlease correct me if I'm wrong, but would a precondtion not just be the postcondition of the setup? Invariants would either have to be publically available and thus easily testable with similar methods, or, one would have to use assertions in the implemention. I try to avoid the latter, as it mixes implemations and 'test/invariants'. Granted, there are situations (usually in code that implements something very 'algorithm'-ish) where inline assertions are so useful that it would be silly to avoid them. (But implementing algos from scratch is rare in commercial code)
- int_19h 4y agoassertions are basically comments with teeth; why would you avoid them in any code, algorithmic or no?
- choeger 4y agoI think you forgot at least one valid assertion and implied another one: foo.call() might have a return value. Also, the whole story invocation shouldn't throw an exception, if your language has them. This assertion is often implied (and that's fine), but it's still there. Finally the test case is a little bit stupid, because very seldom code doesn't have any input that changes the behavior/result. So your assertion would usually involve that input. If you follow that though consequently, you end up with property-based tests very soon. But property-based tests should have as many assertions as possible for a single point of data. Say you test addition. When writing property-based tests you would end up with three specifications: one for one number, testing the identity element and the relationship to increments. Another one for two numbers, testing commutativity and inversion via subtraction, and one for three numbers, testing associativity. In every case it would be very weird to not have all n-ary assertions for the addition operation in the same spot.
- mollerhoj 4y agoWhen you say I 'forgot' an assertion, are you implying that test should include all possible assertions on the code? That would perhaps cover more surface, but my goal (read zealot ideology) here is to have the tests help document the code: test "pressing d key makes mario move 2 pixels right" { expect { keyboard.d() }.to change { mario.x }.by(2) } I could test the value of the d() function, but I dont because I don't care what it returns. Didnt understand the "whole story invocation" and exception part, am I missing some context? Sure property-based testing can be invaluable in many situations. Only downside is if the tests become so complex to reason about that bugs become more likely in the tests than the implemenation. I've sometimes made tests with a manual list of inputs and a list of expected outputs for each. I'd still call that 1 assertion tests (just run multiple times), so my definition of 1 assertion might too broad..
- hinkley 4y agoWhen you get the suites nested and configured right, and the code decomposed properly to support it, each of these assertions is two lines of code, plus the description of each constraint. So you just write four or five tests covering each one, in descending likelihood of breakage.
- AlphaSite 4y agoI think a much better rule of thumb is: “A lot of small unit tests are better than a few big ones”. Same thing, but clearer intent and less rigid.
- hinkley 4y agoUnit tests should be cheap. Cheap to write, cheap to run, cheap to read, cheap to replace. Near as I can tell, many people are made uncomfortable by this in practice because these tests feel childish and dare I say demeaning. So they try to do something “sophisticated” instead which is a slow and lingering death where tests are co corned. Lacking self consciousness, you can whack out hundreds of unit tests in a couple of days, and rewrite ten of someone else’s for a feature or a bug fix. That’s fine and good. But when your test looks like an integration test, rewriting it misses boundary conditions because the test is t clear about what it’s doing. And then you have silent regressions in code with high coverage. What a mess.
- pydry 4y agoThere's a lot of not very competent people in the industry who cling tightly to dogma. Testing (especially unit) is an area of tech weirdly with a lot of dogmatism. I think Uncle Bob is the source of some of it.
- musingsole 4y agoI'm convinced if you read Uncle Bob carefully and follow all his suggestions... you'll have completely incapacitated whatever organization you infiltrated.
- klysm 4y agoThen you need to hire consultants to come fix it!
- drewcoo 4y ago> But that first iteration would never pass code review either because it does nothing and thus is a waste of effort.) That first iteration would not be subject to code review. The author is using TDD. https://en.wikipedia.org/wiki/Test-driven_development https://en.wikipedia.org/wiki/Test-driven_development