subunit-dev team mailing list archive

[Jonathan Lange <jml@xxxxxxxxx>]

On Sat, Dec 5, 2009 at 9:19 PM, Robert Collins
<robertc@xxxxxxxxxxxxxxxxx> wrote:
>
>
>>
>> One test I added was for the behaviour of wasSuccessful(). The test
>> shows that wasSuccessful() returns True, then returns False after
>> addError() is called. I figure this is a valid interface test.
>
>>
>
>> I've made the test pass by having wasSuccessful logic in the wrapper.
>> Personally, I think the logic should be in TestProtocolClient.
>> Otherwise, I don't see why it should even defined as a method at all.
>>
>> Anyway, these are my thoughts.
>
>
> This doesn't say much about motivation: why, in trial
> --reporter=subunit, you would want wasSuccessful to ever return False.
>
> Runners such as trial will call the method all the time - so it has to
> exist. But they will also often try to print failures, and change their
> sys.exit value based on the return value of wasSuccessful, in ways that
> aren't helpful for subunit pipelines or callouts from parent processes.
>
> So I think in a narrow interpretation of the interface, your test is
> valid. In my interpretation, where 'wasSuccessful' means 'should the
> runner execute an error code path', your test isn't valid, because 'the
> runner should not ever execute an error code path *if subunit
> successfully communicated the test activity*.
>

I think the more narrow implementation -- that wasSuccessful()
indicates whether or not the tests passed -- makes more sense, and
allows the executable to decide its own policy on return codes.

I added the test case to stop an actual failure, which escapes me now.
I'm planning on dicking about with the Trial subunit patch tests
tonight (Twisted's reviewers want them exactly their way). This will
help me recall the actual use case.

jml