-
Notifications
You must be signed in to change notification settings - Fork 39
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
Make the sphinx extension testable and more modular #1132
Open
sirosen
wants to merge
18
commits into
globus:main
Choose a base branch
from
sirosen:testable-sphinxext
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
`globus_sdk._sphinxext` will now be a subpackage dir with all of the modules it needs self-contained.
- Add a couple of unit tests guarded with importorskip (in a nice fixture) - Add a relevant tox factor for 'sphinxext' testing - Use that factor in the env_list and CI
The new sphinxext layout, including testability, allows us to better pursue annotation and testing coverage of the sphinx extension code. As such, it is de-listed from coverage and mypy exemptions, and needs to start getting updates, including some minor fixes here.
And update mypy usage to ensure that type checking passes on the newly isolated class. In particular, update typing dependencies to include `sphinx`.
1. Move the directive to a new module, alongside the base AddContentDirective class 2. Add dedicated unit tests for it -- this required expansion of the sphinx extension testing toolchain to cover a full `sphinx` invocation (in-process) I arrived at (2) after trying to integrate the docutils runner I had built with sphinx directly. Sphinx has too many customizations and patches -- the result was imperfect and fragile imitation of sphinx's own internal behaviors.
New unit tests exercise listknownscopes.
2b94681
to
de0bbad
Compare
Move this sphinx directive to a dedicated module and test it from there.
Move these components to dedicated sphinx-extension-specific modules and add unit tests for them.
Move this sphinx directive to a new dedicated module and add some basic unit tests for it.
Move autodoc signature hook to a dedicated module and add unit tests for it.
Move to a dedicated module and add unit tests.
Move the CopyParams directive into the custom directives subpackage, and add tests for it. Additionally, move the param reader utility into the sphinx plugin utility functions, and relocate its (only) unit test.
1. Move `custom_directives` to `directives` to match `roles` 2. Ensure the tests can run on py3.8 -- avoid contextmanager syntax which makes test collection fail 3. Fixup tox.ini to list `-sphinxext` tests in depends 4. Add a hint to the sphinx setup func for mypy
The Windows and macOS builds were failing coverage_report because they were not running the new suite of tests. Additionally, covering these specialized environments ensures a good, uniform development experience.
Requiring an old version of `cryptography` forces an sdist build on the macOS runner, which fails due to OpenSSL version mismatches.
When reading and writing files, don't allow for platform/locale selection of encodings -- explicitly use UTF-8.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is opened as a draft to share my work early, as I make progress on making it the sphinx extension into a suite of well-tested directives.
So far, it does seem necessary to produce a full sphinx run to test things properly, and for now I'm reusing our application's sphinx config.
We could define narrower testing configs, but I'm not sure that's a worthwhile endeavor, since our goal is to make sure we're testing our extension code here.
Type checking is also being addressed module-by-module as I move components around.
We should end up with fully tested and annotated sphinx extension code.
📚 Documentation preview 📚: https://globus-sdk-python--1132.org.readthedocs.build/en/1132/