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

Add test matrix for V3 #1656

Merged
merged 4 commits into from
Feb 12, 2024
Merged

Add test matrix for V3 #1656

merged 4 commits into from
Feb 12, 2024

Conversation

maxrjones
Copy link
Member

@maxrjones maxrjones commented Feb 7, 2024

Part of #1648

The extras are packages that should be optional (with import skipping), which could be addressed separately.

The current setup includes 8 combinations (below), would appreciate feedback on this.

$ hatch env show --ascii
                Standalone                
+---------+---------+----------+---------+
| Name    | Type    | Features | Scripts |
+=========+=========+==========+=========+
| default | virtual |          |         |
+---------+---------+----------+---------+
| docs    | virtual | docs     | build   |
|         |         |          | rtd     |
|         |         |          | serve   |
+---------+---------+----------+---------+
                                        Matrices                                         
+------+---------+---------------------------+----------+----------------+--------------+
| Name | Type    | Envs                      | Features | Dependencies   | Scripts      |
+======+=========+===========================+==========+================+==============+
| test | virtual | test.py3.10-1.24-minimal  | extra    | coverage       | run          |
|      |         | test.py3.10-1.26-minimal  |          | mypy           | run-coverage |
|      |         | test.py3.11-1.24-minimal  |          | pytest         | run-mypy     |
|      |         | test.py3.11-1.26-minimal  |          | pytest-asyncio | run-verbose  |
|      |         | test.py3.10-1.24-optional |          | pytest-cov     |              |
|      |         | test.py3.10-1.26-optional |          |                |              |
|      |         | test.py3.11-1.24-optional |          |                |              |
|      |         | test.py3.11-1.26-optional |          |                |              |
+------+---------+---------------------------+----------+----------------+--------------+

@maxrjones maxrjones changed the title Bootstrap v3 branch with zarrita (#1584) Add test matrix for V3 Feb 7, 2024
@maxrjones
Copy link
Member Author

@jhamman I think this is ready for review if you have time. Mypy predictably fails (#1593), so let me know if you think we should comment that step out in the CI for now.

Copy link
Member

@jhamman jhamman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great @maxrjones. A few suggestions but this is close to going in.

.github/workflows/test-v3.yml Show resolved Hide resolved
hatch run test:run
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if we could use github action's matrix feature here as well. Obviously, we're skipping lots of tests at the moment so running the whole matrix in one action is fine. But as the test suite comes back to life, I think we'll spread out. It seems like Hatch can support this with something like:

hatch run +py=${{ matrix.python-version }} test:run

Note: I wasn't able to figure out how to run individual matrix members with hatch variable selection but it seems like it should be possible: https://hatch.pypa.io/1.9/cli/reference/#hatch-env-run

@jhamman
Copy link
Member

jhamman commented Feb 12, 2024

I'm going to merge this now since I think its a solid improvement.

@maxrjones - if you come back to #1648, we can look into splitting the matrix runs out into a GHA matrix.

@jhamman jhamman merged commit c578963 into zarr-developers:v3 Feb 12, 2024
7 checks passed
@maxrjones
Copy link
Member Author

I'm going to merge this now since I think its a solid improvement.

@maxrjones - if you come back to #1648, we can look into splitting the matrix runs out into a GHA matrix.

Thanks @jhamman! I'll be offline Feb 14 - March 5 and doubt that I will have time to work on this more before then. But, I'll be glad to contribute more after returning if there are still open V3 refactor issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants