Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test elapsed times being significantly under-reported when run in parallel (via Django's --parallel option) #229

Open
bobwhitelock opened this issue Oct 21, 2020 · 5 comments

Comments

@bobwhitelock
Copy link

I have an existing Django project with many tests, and which uses xmlrunner.extra.djangotestrunner.XMLTestRunner from this project as the test runner. I'm using version 3.0.4 of this library on Python 3.7.4.

If I create a file like this in this project:

# notification/tests/test_xmlrunner.py

import time
from django.test import TestCase


class XmlrunnerTest(TestCase):
    def test_wait(self):
        time.sleep(3)

And then run all the tests for this app in serial, I see this result:

$ python manage.py test --keepdb notification
Using existing test database for alias 'default'...
System check identified no issues (0 silenced).

Running tests...
----------------------------------------------------------------------
..............................................................................................................................................
----------------------------------------------------------------------
Ran 142 tests in 59.466s

OK

Generating XML reports...
Preserving test database for alias 'default'...
$ cat TEST-notification.tests.test_xmlrunner.XmlrunnerTest-20201021191521.xml
───────┬───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
       │ File: TEST-notification.tests.test_xmlrunner.XmlrunnerTest-20201021191521.xml
───────┼───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
   1   │ <?xml version="1.0" encoding="UTF-8"?>
   2   │ <testsuite errors="0" failures="0" file="notification/tests/test_xmlrunner.py" name="notification.tests.test_xmlrunner.XmlrunnerTest-20201021191521" s
       │ kipped="0" tests="1" time="3.001" timestamp="2020-10-21T19:16:19">
   3   │     <testcase classname="notification.tests.test_xmlrunner.XmlrunnerTest" file="notification/tests/test_xmlrunner.py" line="8" name="test_wait" time="
       │ 3.001" timestamp="2020-10-21T19:16:19"/>
   4   │ </testsuite>
───────┴───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

I.e. the test is recorded as taking just over 3 seconds, as expected.

If I run the tests in parallel via Django's --parallel option, which will run the tests across all 8 cores of my laptop by default, I see this result:

$ python manage.py test --keepdb notification --parallel
Using existing test database for alias 'default'...
Using existing clone for alias 'default'...
Using existing clone for alias 'default'...
Using existing clone for alias 'default'...
Using existing clone for alias 'default'...
Using existing clone for alias 'default'...
Using existing clone for alias 'default'...
Using existing clone for alias 'default'...
Using existing clone for alias 'default'...
System check identified no issues (0 silenced).

Running tests...
----------------------------------------------------------------------
..............................................................................................................................................
----------------------------------------------------------------------
Ran 142 tests in 30.101s

OK

Generating XML reports...
Preserving test database for alias 'default'...
Preserving test database for alias 'default'...
Preserving test database for alias 'default'...
Preserving test database for alias 'default'...
Preserving test database for alias 'default'...
Preserving test database for alias 'default'...
Preserving test database for alias 'default'...
Preserving test database for alias 'default'...
Preserving test database for alias 'default'...
$ cat TEST-notification.tests.test_xmlrunner.XmlrunnerTest-20201021191736.xml
───────┬───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
       │ File: TEST-notification.tests.test_xmlrunner.XmlrunnerTest-20201021191736.xml
───────┼───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
   1   │ <?xml version="1.0" encoding="UTF-8"?>
   2   │ <testsuite errors="0" failures="0" file="notification/tests/test_xmlrunner.py" name="notification.tests.test_xmlrunner.XmlrunnerTest-20201021191736" s
       │ kipped="0" tests="1" time="0.001" timestamp="2020-10-21T19:17:46">
   3   │     <testcase classname="notification.tests.test_xmlrunner.XmlrunnerTest" file="notification/tests/test_xmlrunner.py" line="8" name="test_wait" time="
       │ 0.001" timestamp="2020-10-21T19:17:46"/>
   4   │ </testsuite>
───────┴───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

I.e. the test is recorded as having run significantly, and impossibly, faster. I see similar behaviour across almost every test in my test suite when I run them using --parallel.

I need to investigate more, but my current guess is that this is due to some interaction between different tests running in parallel, and the start and end times from different tests often end up being used to calculate the elapsed time for tests.

Would be happy to help with a fix, but would be interested if anyone can reproduce/if you know why this is happening/if you have any suggestions for how best to fix? Thanks!

@bobwhitelock
Copy link
Author

I've just seen #228, which looks somewhat related in that it fixes an issue with test times being under-reported. However the fix there doesn't seem to resolve this issue, when I use that branch from my project I still see test runtimes that are too low when I use --parallel.

@bobwhitelock bobwhitelock changed the title Test elapsed times being significantly unreported when run in parallel (via Django's --parallel option) Test elapsed times being significantly under-reported when run in parallel (via Django's --parallel option) Oct 21, 2020
@bobwhitelock
Copy link
Author

bobwhitelock commented Oct 22, 2020

I've had a chance to investigate this issue a bit more, here's some brief notes on what I found:

from django_slowtests.testrunner import DiscoverSlowestTestsRunner
from xmlrunner import XMLTestRunner
from xmlrunner.extra.djangotestrunner \
    import XMLTestRunner as DjangoXMLTestRunner
from xmlrunner.result import _XMLTestResult, _TestInfo


class CustomXMLTestRunner(XMLTestRunner):
    def __init__(self, *args, get_timings, **kwargs):
        super().__init__(*args, **kwargs)
        self.get_timings = get_timings
        self.resultclass = CustomTestResult


class CustomTestResult(_XMLTestResult):
    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)
        self.infoclass = CustomTestInfo

    def generate_reports(self, test_runner):
        timings = dict(test_runner.get_timings())

        # This section is borrowed from
        # `xmlrunner.result._XMLTestResult._get_info_by_testcase`, with just
        # `elapsed_time =` line added.
        for tests in (
                self.successes,
                self.failures,
                self.errors,
                self.skipped,
                self.expectedFailures,
                self.unexpectedSuccesses
        ):
            for test_info in tests:
                if isinstance(test_info, tuple):
                    # This is a skipped, error or a failure test case
                    test_info = test_info[0]

                test_info.elapsed_time = timings[test_info.test_method_name]

        return super().generate_reports(test_runner)


class CustomTestInfo(_TestInfo):
    def __init__(self, test_result, test_method, *args, **kwargs):
        super().__init__(test_result, test_method, *args, **kwargs)
        self.test_method_name = str(test_method)


class CustomTestRunner(DjangoXMLTestRunner, DiscoverSlowestTestsRunner):
    test_runner = CustomXMLTestRunner

    def get_test_runner_kwargs(self):
        super_kwargs = super().get_test_runner_kwargs()
        super_kwargs['get_timings'] = self.get_timings
        return super_kwargs

@bobwhitelock
Copy link
Author

I've logged a Django bug report for the underlying issue here: https://code.djangoproject.com/ticket/32140

@dnozay
Copy link
Member

dnozay commented Nov 16, 2020

Hi @bobwhitelock , it's unclear to me if this is just for tracking or if you are looking for a fix to happen in unittest-xml-reporting. Can you please clarify?

@bobwhitelock
Copy link
Author

@dnozay From my investigation above and the discussion in https://code.djangoproject.com/ticket/32140, I believe this is a bug with the elapsed time reporting of unittest-xml-reporting when tests are run with --parallel, due to an incorrect assumption about how Django will use the custom test result class (that the startTest hook will be called as the test is started, which is true when the tests are run in serial but not when they are run in parallel).

This can be worked around by installing another library (django_slowtests) and using the code above; this was fine for my purposes but isn't ideal and is quite involved. I guess it could also be fixed by making Django conform to this assumption of unittest-xml-reporting, but this sounds like it would be tricky and would probably only fix this for future versions of Django.

It seems to me like it would be best to fix this in this library so it works as expected without workarounds and for all versions of Django. However since a workaround is possible I don't think this is a crucial issue to prioritise, for me at least.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants