Support generating summary reports when using pytest-xdist

[ConorMacBride]

• When mpl-results-path is not set, ensure temp directory is shared across all workers
• When in a worker:
  • Write JSON summaries to {results_dir}/results-xdist-{test_run_uid}-{worker_id}.json
  • Skip generating HTML reports
• When in the controller:
  • Read all JSON summaries {results_dir}/results-xdist-{test_run_uid}-*.json back into current controller process
  • Write merged JSON summaries back to {results_dir}/results.json (if JSON summaries enabled)
  • Generate HTML reports (if HTML summaries enabled)

This likely won't work for remote xdist workers. Maybe we need a way to detect that and warn?

Will publish a new minor release once merged.

TODO

• Merge generated hash libraries together also (and test)

Closes #136
Closes #239

[cvanelteren]

@ConorMacBride nice work! The generated html is empty; fixed it locally

[ConorMacBride]

Thanks @cvanelteren, I'll try to get this tidied up and released. I think I just need to revert the commit that fixes the pybind11 issue; iirc it only affects one minor matplotlib release so likely cleaner to remove.

Do you mean this PR introduces an issue that is causing empty HTML?

[cvanelteren]

Yeah the html was empty for me with the PR. Note that with the applied fixes it was not anymore.

[cvanelteren]

have been testing this last couple of weeks -- must say it is very nice to not have to wait 20 minutes for the visual tests to come in 👍 !

[cvanelteren]

There is a small issue though that pytest-mpl with xdist will cause issue with RcParams being modified. I have noticed some racing conditions when we test on tests that modify RC.

Note that this is ofc not an issue directly with this PR but more in general that users should be aware of this. RcParams is currently not threadsafe in mpl.

[ConorMacBride]

@cvanelteren I wasn't able to reproduce the issue with the empty HTML (337005c). What fix did you apply locally?

That's a good point on RcParams not being thread safe. Don't think there is anything we can really do on this end except document.

[cvanelteren]

The ones I outlined in the review. I can check again in case I was a bit careless.

[cvanelteren]

On b66e2a991de3991097b4516b53975008e895b3e1

[cvanelteren]

Running the same test code with the diff attached creates a complete html:

diff.txt

PS: for reference this is my test command in case that is informative:

pytest -n auto --mpl --mpl-baseline-path=baseline --mpl-default-style="./ultraplot.yml"  --mpl-results-path=results --mpl-generate-summary=html ultraplot/tests

[ConorMacBride]

Thanks @cvanelteren, I didn't check all parts of the diff as there's a lot of reformatting. I did notice the worker vs controller detection has changed. I tried to rely on the xdist functions to tell us which it was, but it sounds like that might not work well across different xdist and pytest versions. I did notice that the xdist controller function was hitting the exception I have accounted for in the code. Might be best to do the detection ourselves.

[cvanelteren]

Ah I see my editor formatted with black the code without me noticing. The main changes are the one I listed that made it work. I just forked it. To see the full context please see: 8a61c20

but you can ignore the formatting stuff, my bad!

[ConorMacBride]

Ah, I see. I applied black to my code and compared to your version to see the differences. I don't think the import changes are needed but I simplified the xdist worker vs controller detection; basically using the short snippet of code that xdist uses itself.

The issue was that the JSON summary needs to always be generated for xdist workers such that the results can be shared across processes and merged back into the results object in the controller. I added a test to catch this.

[cvanelteren]

Exactly! Apologies if this wasn't clear.