test: use pytest-regression in our test suit

jupyter / jupyter-sphinx

Sphinx extension for rendering of Jupyter interactive widgets.

https://jupyter-sphinx.readthedocs.io/

BSD 3-Clause "New" or "Revised" License

187 stars 65 forks source link

test: use pytest-regression in our test suit #265

Closed 12rambau closed 8 months ago

12rambau commented 10 months ago

Fix #122

akhmerov commented 10 months ago

I have a bit mixed feelings about this approach.

Pros:

It is less code
It is easier to add new tests

Cons:

The tests compare a lot more information (the entire HTML) rather than the doctree. This may make tests fail if e.g. sphinx changes its markup.
Perhaps more significantly: verifying that the check is correct requires manual audit of the HTML, and is potentially error prone.

I don't think the drawbacks are severe, but I wanted to bring them up for consideration.

12rambau commented 10 months ago

I also thought about it but I didn't find efficient ways to parse doctree nodes as a json tree which would have allowed me to continue testing the document object.

That being said, the final result is the html output and changes from sphinx needs to be captured very early by the tests to avoid breaking custom css and themes so I think it brings lots of advantages as well. It is still possible to use other builders if the html output is not sufficient or if issues are identified in pdf or epub.

At the moment the PR is not ready for review as the pre-commit hooks are conflicting with the generated files.

12rambau commented 8 months ago

@akhmerov do you have any last though on this one before I merge it ? I would like to have it merged before I merge other PR to avoid the tests misalignment.

akhmerov commented 8 months ago

Sorry for not being clear. I think this is a perfectly fine solution that removes a lot of boilerplate.