dist/testrunner: Capture number of unittests that passed

[leandrolanzieri]

Contribution description

#13642 introduced the check_unittests helper function, to verify the result of unit tests on applications. Some tests (e.g. gnrc_sixlowpan_iphc_w_vrb, gnrc_ipv6_ext_frag) expect the number of passed tests to be captured. This modifies the regex to do that.

Testing procedure

Run tests which make use of this function, they should pass.

Issues/PRs references

#13642

[aabadie]

I would rather use this on the line above: '(\d+)'

[miri64]

This then makes the match object in child.match very inconsistent, depending if nb_tests is provided or not.

[aabadie]

if nb_tests is provided, this is not a regexp anymore.

[miri64]

Of course it is. It just matches to the number provided.

[aabadie]

if you set nb_tests to a value, but the actual number of successful tests is different, the expect will fail anyway. And if it is successful, you already know that number (since it was given as parameter), so there's no need to guess it with a match.

[leandrolanzieri]

ping @aabadie ?

[aabadie]

Still on my position, sorry.

Why do you think your approach is a better approach than having the expect object / side-effects behave consistently on this function call regardless of input?

The problem is not about treating the same way all kind of inputs but check the input to properly handle it (None => regexp, nb_tests => plain string check). If nb_tests is set, you can use the expect_exact match (which I should have done in the first place and which is faster), if not, use expect with a regexp.

[miri64]

I'm fine with you having a position on that. I just don't understand why you want the behavior like that. IMHO it just makes this function harder to understand and use.

[aabadie]

it just makes this function harder to understand and use.

Why ? If it's hard to understand, add comments.
If you want to return the number of successful tests, if nb_tests is None, just return your proposition below (int(child.match(1)), in all other cases, return nb_tests.

[miri64]

Okay, but why add a comment if we can have the same intuitively like this? IMHO we are just bike-shedding. expect_exact should only have minor speed advantages here and the other "advantage" that one can just carelessly copy-paste is not given here, as this function abstracts that away.

[leandrolanzieri]

I modified the function to always return the amount of passed tests. Also changed the tests that use that function, to check for the returned value instead of getting the captured group.

[miri64]

ACK. Fixes some assertions and abstracts away pexpect sufficiently.

[miri64]

Please squash

[aabadie]

Thanks for addressing the changes. However, there's still room for improvement.

[aabadie]

ACK, please squash!

[leandrolanzieri]

I forgot to remove the slashes now that the expect_exact is a string 😅

[leandrolanzieri]

I fixed that and squashed directly.

[leandrolanzieri]

Ok now Murdock is happy