AbdealiLoKo / pytest-attrib

A pytest plugin that mimics the working of nose-attrib
MIT License
3 stars 1 forks source link

Skipping tests that nosetest does not in pywikibot #1

Closed AbdealiLoKo closed 8 years ago

AbdealiLoKo commented 8 years ago

In nose, but not in pytest: {'tests/site_tests:TestImageUsage.test_image_usage_in_redirects', 'tests/site_tests:TestPagePreloading.test_preload_langlinks_count', 'tests/site_tests:TestPagePreloading.test_preload_langlinks_normal', 'tests/site_tests:TestPagePreloading.test_preload_templates', 'tests/site_tests:TestPagePreloading.test_preload_templates_and_langlinks', 'tests/site_tests:TestSiteGenerators.test_allpages_langlinks_enabled'}

AbdealiLoKo commented 8 years ago

The one thing in common that all those tests have is that they have a decorator of allowed_failure_if or allowed_failure

jayvdb commented 8 years ago

Damn, you beat me to it, yes, that will be it.

AbdealiLoKo commented 8 years ago

:D

On Thu, May 19, 2016 at 10:21 PM, John Vandenberg notifications@github.com wrote:

Damn, you beat me to it, yes, that will be it.

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub https://github.com/AbdealiJK/pytest-attrib/issues/1#issuecomment-220385267

AbdealiLoKo commented 8 years ago

It seems as though this is intended pytest behaviour. Essentially, if the decorator raises a SkipTest, pytest considers it deselected. I made a test at https://github.com/AbdealiJK/pytest-attrib/commit/5c09eafef9a1a5ac77ac553791374f97c778ad21#diff-183daaa1d5b2d8847e74fc33c1cf43fa

There is a test for the unittest.skipIf which is True and another test for the unittest.skipIf which is False. When it should be skipped, pytest-attrib does actually select it (debugged using print statements). But pytest finally doesnt consider it selected. I even made a test with -a "1" (Which is always true) to test this.

Hence closing this as it's not the issue of the plugin. Also, it does not cause much harm - as the tests that should run will be run

AbdealiLoKo commented 8 years ago

Reopening as I seem to have been mistaken. The test is skipped (as expected) and should be shown in the travis log. But it's not being shown in the travis log ... so, I didn't recreate the test conditions in that commit correctly.

AbdealiLoKo commented 8 years ago

This seems to be pretty much because of the python3 not having instance methods again ... In python3 the function is defined as <function allowed_failure.<locals>.wrapper at 0xb677765c>. In python2 the function is defined as <unbound method MyTest.wrapper>

Hence, in python3, it doesnt know how to find the parent class for the testcase. I think this could also be claimed as a badly created decorator. Because the decorator unittest.skipIf works fine ...

@jayvdb, in Pywikibot we use (minimal version):

    def allowed_failure(func):
        def wrapper(*args, **kwargs):
            try: func(*args, **kwargs)
            except AssertionError:
                raise unittest.SkipTest('Failing is ok')
        wrapper.__name__ == func.__name__
        return wrapper

this shold preferably be replaced with:

    import functools
    def allowed_failure(func):
        @functools.wraps(func)
        def wrapper(*args, **kwargs):
            try: func(*args, **kwargs)
            except AssertionError:
                raise unittest.SkipTest('Failing is ok')
        return wrapper

As functools is meant for this. And does it in a better (complete?) way

jayvdb commented 8 years ago

If that fixes the problem, then definitely do it. Be sure to check Python 2.6 before claiming success, as that will be were the problem lies. The missing attribute is likely to be __wrapped__, due to laziness. When we are not being lazy, we build our own better version of wraps , but that isnt needed in the test suite, and we probably can just use functools.

AbdealiLoKo commented 8 years ago

Also ran it on travis with pywikibot-core:

Tested the logs using http://pastebin.com/FnMQThYK and found that pytest-attrib only selects more tests than nose-attrib. All of those should be selected, so there seems to be some issue with nose-attr because it doesnt select it.