Closed Bug 1338534 Opened 6 years ago Closed 6 years ago

[mozlog] pytest-mozlog fails with AttributeError when a failure occurs using pytest-xdist

Categories

(Testing :: Mozbase, defect)

Version 3
defect
Not set
normal

Tracking

(firefox54 fixed)

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: davehunt, Assigned: davehunt, Mentored)

References

Details

Attachments

(4 files)

Attached file exception.txt
When running pytest tests with pytest-mozlog and pytest-xdist and a failure is encoutered, an AttributeError occurs as shown in the attachment. This is because pytest-xdist serialises the exception information as a string [1][2].

Steps to replicate:
1. Install mozlog from mozilla-central and pytest from PyPI
2. Run a suite with "-n=1" and a failing test

Expected:
1. Test suite completes without raising any exceptions.

Actual:
1. INTERNALERROR> AttributeError: 'str' object has no attribute 'reprcrash'

[1] https://github.com/pytest-dev/pytest-xdist/blob/f43a64ae592f34215fbbf8642ec30899319ead8c/xdist/remote.py#L102-L105
[2] https://github.com/pytest-dev/pytest/blob/a3319ffe802d532e303b232a961bf63b92ec2144/_pytest/_code/code.py#L652-L668
Depends on: 1338531
Assignee: nobody → dave.hunt
Status: NEW → ASSIGNED
Blocks: 1339462
Comment on attachment 8837114 [details]
Bug 1338534 - [mozlog] Log message and stack for failures when using pytest-mozlog with pytest-xdist.

https://reviewboard.mozilla.org/r/112360/#review113778
Attachment #8837114 - Flags: review?(ahalberstadt) → review+
Comment on attachment 8837115 [details]
Bug 1338534 - [mozlog] Improve handling of expected failures in pytest-mozlog.

https://reviewboard.mozilla.org/r/112362/#review113788

::: testing/mozbase/mozlog/mozlog/pytest_mozlog/plugin.py:74
(Diff revision 1)
> -            if report.skipped:  # indicates expected failure (passing test)
> +        if report.failed:
> -                status = 'FAIL'
> -        elif report.failed:
>              status = 'FAIL' if report.when == 'call' else 'ERROR'
> +        if report.skipped:
> +            status = 'SKIP' if not hasattr(report, 'wasxfail') else 'FAIL'

Why does the status get set to FAIL if there was an expected failure? Shouldn't it stay skipped?
Comment on attachment 8837115 [details]
Bug 1338534 - [mozlog] Improve handling of expected failures in pytest-mozlog.

https://reviewboard.mozilla.org/r/112362/#review113788

> Why does the status get set to FAIL if there was an expected failure? Shouldn't it stay skipped?

A quirk of pytest is that xfailed tests can have the outcome 'skipped' as shown:
https://github.com/pytest-dev/pytest/blob/427bf42a52b524e846bcd2320a511832ac9233f3/_pytest/skipping.py#L246
https://github.com/pytest-dev/pytest/blob/427bf42a52b524e846bcd2320a511832ac9233f3/_pytest/skipping.py#L253

They can also have the outcome of 'failed' as shown:
https://github.com/pytest-dev/pytest/blob/427bf42a52b524e846bcd2320a511832ac9233f3/_pytest/skipping.py#L251
https://github.com/pytest-dev/pytest/blob/427bf42a52b524e846bcd2320a511832ac9233f3/_pytest/skipping.py#L260

If I use 'SKIP' in mozlog, then the expected outcome is stripped, and it's not possible to identify the test as one that was expected to fail: https://dxr.mozilla.org/mozilla-central/rev/0eef1d5a39366059677c6d7944cfe8a97265a011/testing/mozbase/mozlog/mozlog/structuredlog.py#361

An alternative would be for mozlog to permit status 'SKIP' and expected 'FAIL'. This would be distinct from status 'FAIL' and expected 'FAIL' though, which in pytest actually means an unexpected failure occurred on a test that was expected to fail. An example would be expecting a specific exception, but another was thrown.
Comment on attachment 8837115 [details]
Bug 1338534 - [mozlog] Improve handling of expected failures in pytest-mozlog.

https://reviewboard.mozilla.org/r/112362/#review113892

::: testing/mozbase/mozlog/mozlog/pytest_mozlog/plugin.py:74
(Diff revision 1)
> -            if report.skipped:  # indicates expected failure (passing test)
> +        if report.failed:
> -                status = 'FAIL'
> -        elif report.failed:
>              status = 'FAIL' if report.when == 'call' else 'ERROR'
> +        if report.skipped:
> +            status = 'SKIP' if not hasattr(report, 'wasxfail') else 'FAIL'

Looking at this again, the 'skipped' cases are where the expected failure occurs, and the 'failed' cases are where it doesn't (either an unexpected exception is raised, or the test passes). In those 'failed' cases, 'wasxfail' is not set on the report, so our expected outcome will be 'PASS'. I think this covers everything except an unexpected pass when the xfail is strict, which will currently be reported as status='FAIL' and expected='PASS'. This really should be status='PASS' and expected='FAIL', but we will at least have a mismatch, and I believe we'd need a change to pytest for us to detect this case. I'll write to the pytest-dev mailing list to ask about this.
Comment on attachment 8837115 [details]
Bug 1338534 - [mozlog] Improve handling of expected failures in pytest-mozlog.

https://reviewboard.mozilla.org/r/112362/#review113892

> Looking at this again, the 'skipped' cases are where the expected failure occurs, and the 'failed' cases are where it doesn't (either an unexpected exception is raised, or the test passes). In those 'failed' cases, 'wasxfail' is not set on the report, so our expected outcome will be 'PASS'. I think this covers everything except an unexpected pass when the xfail is strict, which will currently be reported as status='FAIL' and expected='PASS'. This really should be status='PASS' and expected='FAIL', but we will at least have a mismatch, and I believe we'd need a change to pytest for us to detect this case. I'll write to the pytest-dev mailing list to ask about this.

Additional patch incoming for strict XPASS.
Comment on attachment 8837115 [details]
Bug 1338534 - [mozlog] Improve handling of expected failures in pytest-mozlog.

https://reviewboard.mozilla.org/r/112362/#review114516
Attachment #8837115 - Flags: review?(ahalberstadt) → review+
Comment on attachment 8837356 [details]
Bug 1338534 - [mozlog] Indicate unexpected pass with the correct status.

https://reviewboard.mozilla.org/r/112504/#review114518
Attachment #8837356 - Flags: review?(ahalberstadt) → review+
Pushed by dhunt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c3aade304f7b
[mozlog] Log message and stack for failures when using pytest-mozlog with pytest-xdist. r=ahal
https://hg.mozilla.org/integration/autoland/rev/501c0a140c82
[mozlog] Improve handling of expected failures in pytest-mozlog. r=ahal
https://hg.mozilla.org/integration/autoland/rev/b854735013bb
[mozlog] Indicate unexpected pass with the correct status. r=ahal
Blocks: 1301494
You need to log in before you can comment on or make changes to this bug.