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: switch from sphinx-testing @with_app to sphinx.application.Sphinx #155

Merged
merged 1 commit into from
May 7, 2023

Conversation

jnikula
Copy link
Owner

@jnikula jnikula commented May 4, 2023

The sphinx-testing package that provides the handy @with_app decorator has been deprecated in favour of sphinx.testing for years.

The sphinx.testing package provides a make_app pytest fixture to create a test app, but there are a number of downsides with it:

  • You can only use make_app as a pytest fixture, and for test functions only. In our case, we'd have to route make_app as a parameter from test_directive_*() via testcase.run_test() all the way to the get_output() and get_expected() methods.

  • There's no automatic temp directory creation and deletion for srcdir.

  • There's no confdir parameter, so conf.py needs to be copied.

  • It forces you to use the weird sphinx.testing custom path class.

I fail to see the upsides of jumping through hoops to use sphinx.testing for testing. Turns out it's actually much more clear to use sphinx.application.Sphinx directly. A lot of the "helpful" parts that make_app provides turn out to be something that we'd need to work around, and the Sphinx app lets us clean up things that were needed because of @with_app.

Use sphinx.application.Sphinx directly for Sphinx build.

The sphinx-testing package that provides the handy @with_app decorator
has been deprecated in favour of sphinx.testing for years.

The sphinx.testing package provides a make_app pytest fixture to create
a test app, but there are a number of downsides with it:

- You can only use make_app as a pytest fixture, and for test functions
  only. In our case, we'd have to route make_app as a parameter from
  test_directive_*() via testcase.run_test() all the way to the
  get_output() and get_expected() methods.

- There's no automatic temp directory creation and deletion for srcdir.

- There's no confdir parameter, so conf.py needs to be copied.

- It forces you to use the weird sphinx.testing custom path class.

I fail to see the upsides of jumping through hoops to use sphinx.testing
for testing. Turns out it's actually much more clear to use
sphinx.application.Sphinx directly. A lot of the "helpful" parts that
make_app provides turn out to be something that we'd need to work
around, and the Sphinx app lets us clean up things that were needed
because of @with_app.

Use sphinx.application.Sphinx directly for Sphinx build.
@BrunoMSantos
Copy link
Collaborator

Hmm, when I looked at it I got the impression it would be tricky to convert, but this is pretty simple. Looks good to me ;)

Sorry I only got around to it now.

@jnikula
Copy link
Owner Author

jnikula commented May 7, 2023

Hmm, when I looked at it I got the impression it would be tricky to convert, but this is pretty simple. Looks good to me ;)

It would've been tricky to convert to sphinx.testing! Just using the actual Sphinx app directly was much easier.

Sorry I only got around to it now.

No worries, and thanks!

@jnikula jnikula merged commit 0c4e4f8 into master May 7, 2023
4 checks passed
@jnikula jnikula deleted the sphinx-testing branch July 4, 2023 08:59
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

Successfully merging this pull request may close these issues.

None yet

2 participants