Code review comment for lp:~yellow/subunit/test-count

Revision history for this message
Gary Poster (gary) wrote :

Hi Jono.

While this change is stand-alone from subunit's perspective, we actually were working with https://code.launchpad.net/~jml/subunit/filter-tags/+merge/102840 .

Without the filter-tags branch, http://pastebin.ubuntu.com/950663/ is a sufficient change to get the testrepository tests to pass with subunit's trunk (after your already-completed merge of https://code.launchpad.net/+branch/~yellow/subunit/real-time).

However, with the filter-tags branch and the testrepository patch above, we still need to address additional failures: http://pastebin.ubuntu.com/951133/ . Those are the failures that this branch tried to fix.

The _PredicateFilter refactoring in the filter-tags branch makes it harder to come up with a solution. The _filtered override changes you suggested will break just as much as the existing _filtered method override does. If instead we override addFailure, addError, and addSkip to increment testsRun there, we double count tests that matched the filter.

I think the best approach will be a subunit change like the one you gave...except that I think the argument will need to be something on the order of "on_filter," because we need to increment testsRun manually only when a test is filtered out in order to get the behavior testrepository desires. I'll make a new branch that does that.

Thanks,

Gary

« Back to merge proposal