Reorganize the tests into unit/functional/integration/external #1155

mwouts · 2023-11-07T21:42:23Z

This is very much a work in progress for now.
I'll try to classify all the different tests.
Will close #1136.

codecov · 2023-11-07T22:12:05Z

Codecov Report

Merging #1155 (3fcc046) into main (4b56512) will increase coverage by 1.96%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1155      +/-   ##
==========================================
+ Coverage   93.12%   95.08%   +1.96%     
==========================================
  Files          29       29              
  Lines        4419     4418       -1     
==========================================
+ Hits         4115     4201      +86     
+ Misses        304      217      -87

Files	Coverage Δ
src/jupytext/parse_version.py	`100.00% <100.00%> (+80.00%)`	⬆️

... and 10 files with indirect coverage changes

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

mwouts · 2023-11-09T22:58:38Z

Hi @LecrisUT , were you thinking of something like the current PR? You also suggested to add pytest markers I guess? And maybe it would be good to check that the test dependencies I have described at 30bd073 are indeed correct - is this something you expect to see on the CI in some way?

In order to keep the package size under control when tests are included

LecrisUT · 2023-11-12T10:48:25Z

Sorry, had a long week, I have a few notes, let me get to my PC later today and I'll write them out

mwouts · 2023-11-12T11:17:11Z

No worries at all! Take your time! This can as well be later on this week or next week-end. Thanks!

LecrisUT

Sorry for the late review. Overall 👍 just 2 things to addressed in this PR:

Where to put the black, etc. dependencies. test-integration or test-external. Having them in test_functional might be a bit complicated for packaging
Using other methods to pytest.skip

LecrisUT · 2023-11-13T08:16:38Z

pyproject.toml

+  "autopep8",
+  "black",
+  "isort",
+  "flake8",
+  # jupytext --execute
+  "ipykernel",
+  # Pre-commit tests
+  "gitpython",
+  "pre-commit",
+  # Interaction with other contents managers
+  # jupyter-fs==0.4.0 is async, which is not supported by Jupytext ATM
+  "jupyter-fs<0.4.0"


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 the optional-dependency (those that are optional) and keep the unit/integration/functional` categorization like this.

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 and integration 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?)

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 is demo which is good to include as well

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.

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.

Just quick links from my projects:

.codecov.yaml: https://github.com/spglib/spglib/blob/b02dedb2eb4b5700ab75c7057f4e7d4d2f38acf8/.codecov.yaml#L14-L31

Github action: https://github.com/spglib/spglib/blob/b02dedb2eb4b5700ab75c7057f4e7d4d2f38acf8/.github/workflows/step_test.yaml#L103-L109

Nice! Thanks for sharing! I'll try this asap.

pyproject.toml

tests/conftest.py

tests/functional/cli/test_black.py

tests/functional/cli/test_cli.py

unit and functional (and even integration) tests run with very little dependencies

LecrisUT · 2023-11-20T05:29:11Z

.github/workflows/step_tests_categories-pip.yml

@@ -0,0 +1,50 @@
+name: test-categories


These need to be added to .github/workflows/ci.yaml.
Also, remind me to make a PR to re-organize these CI files.

LecrisUT · 2023-11-20T05:50:36Z

About the conda test failure, it seems you need to add a browser in the relevant packages.json

src/jupytext/parse_version.py

mwouts · 2023-11-23T21:28:30Z

Thanks @LecrisUT for guiding me into this! I am very glad with the outcome.

I've set up the coverage flags:

Also the coverage seems to go up (but I would like to see codecov commenting on this PR too!)

Also, remind me to make a PR to re-organize these CI files.

Thanks, they are all yours!

mwouts · 2023-11-23T22:32:47Z

@kloczek I know from #906 that the tests/packaging matter to you. We have recently changed both the packaging and the tests, would you mind to have a look and let us know if everything seems alright to you?

kloczek · 2023-11-24T00:09:19Z

@kloczek I know from #906 that the tests/packaging matter to you. We have recently changed both the packaging and the tests, would you mind to have a look and let us know if everything seems alright to you?

No problem. 😄
Jut please let me know what you want to me to test?

mwouts self-assigned this Nov 7, 2023

mwouts mentioned this pull request Nov 9, 2023

JupyterLab Extension to Launch Text Notebooks #1154

Merged

mwouts force-pushed the reorganize_tests branch from d070bb6 to 6a7015c Compare November 9, 2023 22:33

mwouts force-pushed the reorganize_tests branch 2 times, most recently from 54ea28e to ff68ecd Compare November 11, 2023 22:41

mwouts changed the title ~~WIP Reorganize tests~~ Reorganize the tests into unit/integration/functional Nov 11, 2023

mwouts added 9 commits November 12, 2023 10:16

Move the test notebooks to tests/data/notebooks

2493743

Move the pre-commit tests to a subdir

555d0d7

Split tests into unit/integration/functional

eb0e77e

Split the test dependencies into test/integration/functional

2ef289a

Document test requirements with pytest markers

32f8c92

Use packaging.version if available

c1850be

Ignore a few more warnings

e22b7be

Move the codecov.yml configuration file to .github

c442e3f

Remove the world population notebook from the examples

3fcc046

In order to keep the package size under control when tests are included

mwouts force-pushed the reorganize_tests branch from ff68ecd to 3fcc046 Compare November 12, 2023 10:16

mwouts mentioned this pull request Nov 12, 2023

Split tests into unit, integration, functional #1136

Closed

LecrisUT suggested changes Nov 13, 2023

View reviewed changes

mwouts added 2 commits November 19, 2023 17:05

Move some tests to external

4b3eec3

unit and functional (and even integration) tests run with very little dependencies

Test unit/functional/integration/external tests and their dependencies

bf6b3e4

mwouts force-pushed the reorganize_tests branch from 1f55467 to bf6b3e4 Compare November 19, 2023 17:05

mwouts added 5 commits November 19, 2023 21:59

Parametrize the tests by all the Python files in src/jupytext

0b9344d

Move tests with dependencies to integration/external

f180532

Move test on pre-commit mode

3d5b528

Label as external a test on the rst2md option

ab15185

Fix import error

a6ca72d

mwouts added 4 commits November 19, 2023 23:05

Filter more of the expected warnings

57ed3b4

Install JupyterLab before testing the extension

7442c15

Remove isort_version

8f29535

Remove matrix.external, kernel=true is the default

674b3aa

mwouts changed the title ~~Reorganize the tests into unit/integration/functional~~ Reorganize the tests into unit/functional/integration/external Nov 19, 2023

mwouts added 4 commits November 19, 2023 23:41

Install external dependencies too

54189af

Install JupyterLab before running UI tests

096ff0d

Fix name of env

abc6676

Restore environment-ci.yml

3b82bdf

mwouts force-pushed the reorganize_tests branch from 8db6cae to 3b82bdf Compare November 20, 2023 00:26

LecrisUT reviewed Nov 20, 2023

View reviewed changes

src/jupytext/parse_version.py Outdated Show resolved Hide resolved

mwouts added 4 commits November 21, 2023 21:42

Include step test_classification in the CI

595e154

Remove empty line

1417ca7

Turn the tests notebooks into fixtures

a2cd5c7

packaging is a dependency

23fdd2d

mwouts force-pushed the reorganize_tests branch from 7867bae to 23fdd2d Compare November 23, 2023 18:49

mwouts added 7 commits November 23, 2023 18:56

Importing parse won't fail

12b2201

Try codecov flags

6e10678

Remove marker 'requires_jupytext'

6ea8a80

Simplify the pytest command

12dbf54

Move some tests to external

4fb0dba

Rename test to avoid conflict

3c8608d

Fix the tests on Windows

52c53b1

mwouts merged commit 923798e into main Nov 23, 2023
23 of 25 checks passed

mwouts deleted the reorganize_tests branch November 23, 2023 21:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reorganize the tests into unit/functional/integration/external #1155

Reorganize the tests into unit/functional/integration/external #1155

mwouts commented Nov 7, 2023

codecov bot commented Nov 7, 2023 •

edited

Loading

mwouts commented Nov 9, 2023

LecrisUT commented Nov 12, 2023

mwouts commented Nov 12, 2023

LecrisUT left a comment •

edited

Loading

LecrisUT Nov 13, 2023

mwouts Nov 19, 2023

LecrisUT Nov 20, 2023

LecrisUT Nov 20, 2023 •

edited

Loading

mwouts Nov 20, 2023

LecrisUT Nov 20, 2023

mwouts Nov 21, 2023

LecrisUT Nov 20, 2023

LecrisUT commented Nov 20, 2023

mwouts commented Nov 23, 2023

mwouts commented Nov 23, 2023

kloczek commented Nov 24, 2023

Reorganize the tests into unit/functional/integration/external #1155

Reorganize the tests into unit/functional/integration/external #1155

Conversation

mwouts commented Nov 7, 2023

codecov bot commented Nov 7, 2023 • edited Loading

Codecov Report

mwouts commented Nov 9, 2023

LecrisUT commented Nov 12, 2023

mwouts commented Nov 12, 2023

LecrisUT left a comment • edited Loading

Choose a reason for hiding this comment

LecrisUT Nov 13, 2023

Choose a reason for hiding this comment

mwouts Nov 19, 2023

Choose a reason for hiding this comment

LecrisUT Nov 20, 2023

Choose a reason for hiding this comment

LecrisUT Nov 20, 2023 • edited Loading

Choose a reason for hiding this comment

mwouts Nov 20, 2023

Choose a reason for hiding this comment

LecrisUT Nov 20, 2023

Choose a reason for hiding this comment

mwouts Nov 21, 2023

Choose a reason for hiding this comment

LecrisUT Nov 20, 2023

Choose a reason for hiding this comment

LecrisUT commented Nov 20, 2023

mwouts commented Nov 23, 2023

mwouts commented Nov 23, 2023

kloczek commented Nov 24, 2023

codecov bot commented Nov 7, 2023 •

edited

Loading

LecrisUT left a comment •

edited

Loading

LecrisUT Nov 20, 2023 •

edited

Loading