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

RESOLVED FIXED in Firefox 54

Status

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: davehunt, Assigned: davehunt, Mentored)

Tracking

Version 3
mozilla54
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox54 fixed)

Details

Attachments

(4 attachments)

Created attachment 8836020 [details]
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
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Assignee: nobody → dave.hunt
Status: NEW → ASSIGNED

Comment 3

2 years ago
mozreview-review
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 4

2 years ago
mozreview-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?
(Assignee)

Comment 5

2 years ago
mozreview-review-reply
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.
(Assignee)

Comment 6

2 years ago
mozreview-review
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.
(Assignee)

Comment 7

2 years ago
mozreview-review-reply
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 hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 12

2 years ago
mozreview-review
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 13

2 years ago
mozreview-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+

Comment 14

2 years ago
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

Comment 15

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c3aade304f7b
https://hg.mozilla.org/mozilla-central/rev/501c0a140c82
https://hg.mozilla.org/mozilla-central/rev/b854735013bb
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.