✅ test: add tests & CI #18
Conversation
jorenham
added
✅ tests
.github/workflows/ci.yml
Outdated
|
|
||
| - name: Install the project | ||
| run: uv sync --group test |
There was a problem hiding this comment.
I don't think that --group plays well with caching, because we use different groups in different jobs with the same cache key. Uv is already insanely fast, so I don't expect that using different cache keys will help much in practice. So I suppose we could just install everything, and skip this step altogether.
| - name: Install the project | |
| run: uv sync --group test |
There was a problem hiding this comment.
I see.
The uv docs are pretty self-contradictory in terms of advice for best practices.
Following https://docs.astral.sh/uv/guides/integration/github/#multiple-python-versions, what I have now is good.
Can we run down the minutiae in a followup PR? This setup follows recommendations and works, but may not be 100% what we want.
nstarman
force-pushed
the
tests
branch
3 times, most recently
from
542d303 to
be6ff4e
Compare
|
I'd like to figure out how to get the CI going to test the suggested changes to the CI! (I know the current setup works) |
Signed-off-by: Nathaniel Starkman <[email protected]>
nstarman
force-pushed
the
tests
branch
2 times, most recently
from
6c9a852 to
20159bb
Compare
tests/test_namespace.py
Outdated
There was a problem hiding this comment.
I don't love this test. xpt.HasArrayNamespace is final and not runtime-checkable. This is a bit of a hack.
There was a problem hiding this comment.
We could also rm this, and leave it to the integration tests in a follow-up, AFAIK.
There was a problem hiding this comment.
I do want to refactor this, but for the moment working tests are better than None 🤷 .
I think the way we want to refactor this might be to something similar to https://github.com/python/mypy/tree/master/test-data/unit
There was a problem hiding this comment.
I think the way we want to refactor this might be to something similar to python/mypy@
master/test-data/unit
That looks horrible haha. I'd prefer a more IDE-friendly approach like https://github.com/numpy/numtype/tree/main/src/numpy-stubs/%40test , possibly with automatic test-generation like numtype also does.
In https://github.com/scipy/scipy-stubs/tree/master/tests I explained the mechanisms of this testing method a bit better.
There was a problem hiding this comment.
SGTM! Want to open a PR for that?
There was a problem hiding this comment.
Yea, sure. But it probably won't be today or tommorrow.
nstarman
force-pushed
the
tests
branch
6 times, most recently
from
a569ad9 to
4b77c67
Compare
tests/test_namespace.py
Outdated
There was a problem hiding this comment.
We could also rm this, and leave it to the integration tests in a follow-up, AFAIK.
| env: | ||
| # Many color libraries just need this to be set to any value, but at least | ||
| # one distinguishes color depth, where "3" -> "256-bit color". | ||
| FORCE_COLOR: 3 |
There was a problem hiding this comment.
we could add UV_LOCKED: 1 here, so that we don't have to use --locked and --frozen anymore in the uv commands
There was a problem hiding this comment.
Does that work when we have the --resolution lowest-direct?
Since the CI isn't on (hmm...) it's hard to tell.
There was a problem hiding this comment.
I'm not sure if --resolution lowest-direct is the best way to go here actually. Maybe I'm wrong here, but I thought that SPEC 0 applied to the latest patch release of a minor version.
There was a problem hiding this comment.
We still want to test the oldest versions of all supported dependencies.
Co-authored-by: Joren Hammudoglu <[email protected]> Signed-off-by: Nathaniel Starkman <[email protected]>
Requires #17