Hi Chris,

> testMethodRegisteredAsAnExpectedFailure
> self assert: (falseExpression). "A"
> self assert: (trueExpression). "B"

Implemented like this, one should consider this as *one test* -- regardless of the number of assertions inside. Still, one might want to refactor it and swap the two lines to simplify debugging. However, that's only useful if the semantic order of both assertions got mixed up in the first place. It might have anything to do with the actual cause of the failure.

Still, one could *add* another test that focuses on the failing assertion -- if possible:

testMethodRegisteredAsAnotherExpectedFailure
   self assert: (falseExpression). "A"


Makes it easier for the programmer to figure out what to fix first in the future. Both tests would be expected failures, though.

> But expectedFailures are a subset *within* a domain-specific reason that
> not only *should* (IMO), but *need* to be separated out, because SUnit
> provides registration of expectedFailures ONLY at the #test... method
> level.


Hmm... no. Given that expected failures work at the level of a #test... method, programmers can just work with that. Actually, SUnit relies on the programmer to figure out the best granularity for both passing and failing tests. There is no need to do some analysis that goes sub-method here. 

***

Anyway, what you propose here introduces a *new* feature to SUnit. It does *not fix* an existing one. :-) Regarding that proposal, I think that #shouldPass should not be called during test runs so frequently as in an #assert: call. I consider #shouldPass kind of a documentation used for tools such as in the test runner. Your proposed change in TestResult >> #unexpectedPasses renders the code practically unreadable:

unexpectedPasses
  ^ failures select: [:each | each shouldPass not]

What does it say? Well, somehow, a "pass" got stored in the variable "failures" and if you happen to know that #shouldPass can sort out those hidden passes... way too much magic.

This was your initial fallacy for this proposal:

> The Result of a Test which is tagged as an #expectedFailure IS a failure if the test passed.

No, it is not. :-) If anything, the programmer could be happy that an expected failure passes. It is *not a failure*. Maybe something third, which we don't have yet. Serendipity? Happy accident? Not a failure.

Best,
Marcel
Am 27.05.2019 21:07:23 schrieb Chris Muller <asqueaker at gmail.com>:
Hi Marcel,

This change simply fixes the display for exclusion (or inclusion)
of "expectedFailures" in the list, and nothing more. It does *not*
change granularity of assert: at all.

The way tests are written is unchanged, and still up to the test
developer. If you don't use #expectedFailures, you can ignore this
completely. That's the only use-case being fixed here.

> yes, tests should be as modular and concise as possible -- only checking for "one thing" in each test. However, #assert: is *not* the level of granularity one should look at to judge a certain test's modularity. That is, there can be more than one #assert: in a test and still the test be regarded as testing "one thing".
>
> So... I do not quite agree with your statement here ... but I think you mean the right thing:
>
> > Expected failures should be put into their own #test.. method
> > and not mixed with assertions that are expected to pass. Otherwise,
> > the SUnit domain won't be able to properly express to you the state of
> > those tests, as you said.
>
> One should not compose tests according to pass-or-not but rather following domain/project-specific reasons.

Agree, and they still are composed for domain-specific reasons. But
expectedFailures are a subset *within* a domain-specific reason that
not only *should* (IMO), but *need* to be separated out, because SUnit
provides registration of expectedFailures ONLY at the #test... method
level.

So nothing is changed w.r.t. this issue you raise about mixing
expectedFailures with expectedPasses. Doing so today would result in
the same ambiguity within the single test, with or without this
change. Like, what would you think if you came upon this?

testMethodRegisteredAsAnExpectedFailure
self assert: (falseExpression).
self assert: (trueExpression)

The test is registered as an expectedFailure, so regardless of whether
you classify that TestResult as "1 unexpected passes", "1 expected
failure", either way, you have hidden information from yourself about
the OTHER test in that method. This change has nothing to do with
that.

> Then, I know about some tests that put a guard in the beginning of a test to provide a useful "failure message" to the user before running into some other assertion failures.

expectedFailures is a feature of SUnit designed to handle use cases
like the one I described.

> Still not so sure about this hook, though, ... hmmm...

It's not a "hook" as much as a correction to whether a particular
failed assertion should be expected or not.

Best,
Chris
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20190528/896727c5/attachment.html>