-
Notifications
You must be signed in to change notification settings - Fork 385
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
Reorganize the tests into unit/functional/integration/external #1155
Merged
Merged
Changes from 9 commits
Commits
Show all changes
37 commits
Select commit
Hold shift + click to select a range
2493743
Move the test notebooks to tests/data/notebooks
mwouts 555d0d7
Move the pre-commit tests to a subdir
mwouts eb0e77e
Split tests into unit/integration/functional
mwouts 2ef289a
Split the test dependencies into test/integration/functional
mwouts 32f8c92
Document test requirements with pytest markers
mwouts c1850be
Use packaging.version if available
mwouts e22b7be
Ignore a few more warnings
mwouts c442e3f
Move the codecov.yml configuration file to .github
mwouts 3fcc046
Remove the world population notebook from the examples
mwouts 4b3eec3
Move some tests to external
mwouts bf6b3e4
Test unit/functional/integration/external tests and their dependencies
mwouts 0b9344d
Parametrize the tests by all the Python files in `src/jupytext`
mwouts f180532
Move tests with dependencies to integration/external
mwouts 3d5b528
Move test on pre-commit mode
mwouts ab15185
Label as external a test on the rst2md option
mwouts a6ca72d
Fix import error
mwouts 4e32850
Include tests in the Jupytext package
mwouts 4406786
Use Python 3.x for the external tests
mwouts 57ed3b4
Filter more of the expected warnings
mwouts 7442c15
Install JupyterLab before testing the extension
mwouts 8f29535
Remove isort_version
mwouts 674b3aa
Remove matrix.external, kernel=true is the default
mwouts 54189af
Install external dependencies too
mwouts 096ff0d
Install JupyterLab before running UI tests
mwouts abc6676
Fix name of env
mwouts 3b82bdf
Restore environment-ci.yml
mwouts 595e154
Include step test_classification in the CI
mwouts 1417ca7
Remove empty line
mwouts a2cd5c7
Turn the tests notebooks into fixtures
mwouts 23fdd2d
packaging is a dependency
mwouts 12b2201
Importing parse won't fail
mwouts 6e10678
Try codecov flags
mwouts 6ea8a80
Remove marker 'requires_jupytext'
mwouts 12dbf54
Simplify the pytest command
mwouts 4fb0dba
Move some tests to external
mwouts 3c8608d
Rename test to avoid conflict
mwouts 52c53b1
Fix the tests on Windows
mwouts File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
There are no files selected for viewing
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,7 +13,7 @@ | |
exclude: > | ||
(?x)^( | ||
demo/.*| | ||
tests/notebooks/.*| | ||
tests/data/notebooks/.*| | ||
)$ | ||
repos: | ||
|
||
|
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
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,10 @@ | ||
def parse_version(version, custom_error=ImportError): | ||
try: | ||
from pkg_resources import parse_version as parse | ||
from packaging.version import parse | ||
mwouts marked this conversation as resolved.
Show resolved
Hide resolved
|
||
except ImportError: | ||
try: | ||
from packaging.version import parse | ||
from pkg_resources import parse_version as parse | ||
except ImportError: | ||
raise custom_error("Please install either pkg_resources or packaging") | ||
raise custom_error("Please install either packaging or pkg_resources") | ||
|
||
print(version) | ||
return parse(version) |
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
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer thse in an integration test, mostly because these are optional dependencies, and they primarily test how
jupytext
interacts with other tools. Indeed the tests are better classified as functional tests, and I see many projects do so, so it is more of a subjective classification.Why I would prefer this as
integration
is because w.r.t. packaging, unit and functional tests are good to include as a whole, while integration tests are a bit hit-and-miss (mostly because most projects use mocking for these, and the package versions can differ drastically with the one tested upstream).We could also move these to
test-external
as theoptional-dependency
(those that are optional) and keep theunit
/integration
/functional` categorization like this.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the guidance! Actually I have moved all the tests that involve a non-core dependency to
tests/external
.All the tests in
unit
,functional
andintegration
pass, with very little dependencies. I think all these tests should always pass.I have also included the tests in the package (but I was not sure whether
tests/external
should be included too?)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
About packaging, yes it is better to be safe and include everything. Also,
sdist
only picks up on git sources (non-git sources are added as artifacts), and the only other one in the list isdemo
which is good to include as wellThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
About the tests 👍 to the organization. Just a comment, probably you are aware of the practice of mirroring the source files with the test files, but that is only meaningful in unit-tests, which you seem to be following, so 👍. Also, a coding practice tip, it is good to split the codecov flags into unit-test and the rest.
There is one confusing part, why areall of the tests as python packages? Pytest should automatically resolve them from the
test_*
naming scheme.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @LecrisUT !
Can you point me to an exemple re the differentiated coverage ?
Re the inits in tests, indeed I am planning to remove them in this PR. Right now I need them because they import from utils.py but I do plan to use fixtures instead of list_notebooks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just quick links from my projects:
.codecov.yaml
: https://github.com/spglib/spglib/blob/b02dedb2eb4b5700ab75c7057f4e7d4d2f38acf8/.codecov.yaml#L14-L31There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Thanks for sharing! I'll try this asap.