[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Skip-XFail tests accidentally reported as "PASS"

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Fri, 23 May 2008 11:48:30 +0100

Branko Čibej wrote:
> Julian Foad wrote:
>
>> Our test suite can report an XFAIL test as "PASS". This happens when a
>> test is declared as "Skip(XFail(...".
>>
>> The bug appears to be that the Skip class fails to delegate its
>> "run_text()" method to the nested class. I think the attached patch
>> fixes it. The only immediate effect it has on the basic test suite
>> (ra_local) running on my Linux system is that it changes:
>>
>> PASS: merge_tests.py 85: merge --reintegrate should fail on stale
>> source
>>
>> to the correct result:
>>
>> XFAIL: merge_tests.py 85: merge --reintegrate should fail on stale
>> source
>>
>> Is there anyone who could review this, please? Preferably somebody who
>> understands inheritance in Python classes.
>>
>>
>> FYI, tests that depend on both XFail and Skip/SkipUnless are:
>> [[[
>> $ (cd subversion/tests/cmdline/ && grep -i "XFail.*Skip" *.py)
>> authz_tests.py: XFail(SkipUnless(authz_svnserve_anon_access_read,
>
>
> [...]
>
>> prop_tests.py: Skip(XFail(revprop_change,
>> svntest.main.is_ra_type_dav),
>
>
> [...]
>
> Am I right that XFail(Skip(...)) works, but Skip(XFail(...)) doesn't? If
> so, we could just forbid the latter ... after all, assuming that
> everything works, there's no semantic difference.

Yes, that's right. But is there any reason to prefer forbidding the latter (and
how would we publicise or enforce that?) versus fixing the classes so they work
either way around, given that it appears to be easy to fix?

>> I also noticed how Skip and XFail handle the _list_mode_text member in
>> two different ways - neither of them by overriding the list_mode()
>> method. Is this why "authz_tests.py list" shows "XFAIL" for 13/14
>> while the result of running them is "SKIP", or "merge_tests.py list"
>> shows blank for 85 while the result is "XFAIL"?
>
> Showing XFAIL in the list where the particular result on the particular
> platform is SKIP is what I originally intended. The XFAIL is a lot more
> interesting to someone who just wants to list the available tests in a

That makes some sense. I might see if I can make that work properly for the
Skip(XFail()) case.

> suite; the SKIP can depend on lots of parameters, not just the platform.

XFail can depend on lots of parameters too: it takes a condition argument, just
like Skip does. This seemed a bit surprising when I first noticed, but makes
sense. I don't know if it originally did take a condition.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-05-23 12:48:46 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.