Enhance --build-deps-for wheel test#2336
Conversation
A test for `--build-deps-for` was not setting `--allow-unsafe`, and as a result was not checking the version of the build backend (`setuptools`) being fetched/installed. Make the test a better exercise of this functionality by - pinning the `setuptools` version in `build-system.requires` - passing `--allow-unsafe` and then checking the result.
webknjaz
left a comment
webknjaz
left a comment
There was a problem hiding this comment.
I left some ideas inline but won't insist on them.
Side note: do we have a test with an in-tree build backend returning setuptools as a dynamically injected dep through a PEP 517/660 hook? I have those in ansible-pylibssh, yarl and multidict, I think.
| setuptools_version, | ||
| ): | ||
| (tmp_path / "pyproject.toml").write_text(dedent(""" | ||
| (tmp_path / "pyproject.toml").write_text(dedent(f""" |
There was a problem hiding this comment.
Side note: should we have a helper (a separate effort) that would write TOML files from a dict rather than working with strings, which seems a bit fragile?
There was a problem hiding this comment.
👍 tomli-w or tomlkit could do it.
Maybe a good-first-issue, but I'd first want to get the LLM policy done -- those issues seem to be a magnet for slop.
tests/test_cli_compile.py
Outdated
There was a problem hiding this comment.
Should we add these comments as ids?
| ( | ||
| "82.0.0", # latest as of 2026-02-11 | ||
| "75.3.0", # latest as of 2025-01-01 | ||
| ), |
There was a problem hiding this comment.
| ), | |
| ), | |
| ids=('82.0.0-released-on-2026-02-11', '75.3.0-released-on-2025-01-01'), |
There was a problem hiding this comment.
(though, we could also have a data class w/ version + date fields and __str__(), then use ids=str instead)
There was a problem hiding this comment.
I like the dataclass idea. I've been meaning to write a post to socialize the idea of doing that with ids=str -- it's a great pattern, IMO.
I'll also need to actually get those release dates -- I was taking "the latest on $date", but it's probably better to record the exact release dates.
tests/test_cli_compile.py
Outdated
There was a problem hiding this comment.
I'd prefer to keep the build-system section at the top. This seems more conventional.
I don't think we do. I don't remember creating such a test, so it would have to have been added before my time, and we've only discussed ideas around testing with custom build backends recently. I also want to do testing with fully custom build backends. My plan is to work on single-file ones we can vendor into the testsuite -- I think that would let us test a lot of behaviors without needing to pull in |
`pytest` checks to see if this class can be treated as a test class under default rules because its name started with `Test`. This results in warnings in test output, and can be trivially fixed by renaming the class. `...Param` is potentially a good naming scheme for these dataclasses in general (see also jazzband#2336). It connects to `parametrize` usage well.
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
2 participants
This is a very small update inspired by a close examination of #2105.
I consider it a minor improvement, hence no changelog.
This differs from the changes in #2105, as I believe that has drifted too far from our current status.
In particular, we now treat
pyproject.tomldata as static when possible, so we aren't invoking build backends in the same way.A test for
--build-deps-forwas not setting--allow-unsafe, and as aresult was not checking the version of the build backend (
setuptools)being fetched/installed.
Make the test a better exercise of this functionality by
setuptoolsversion inbuild-system.requires--allow-unsafeand then checking the result.
Contributor checklist
changelog.d/(seechangelog.d/README.mdfor instructions) or the PR text says "no changelog needed".
Maintainer checklist
bot:chronographer:skiplabel.(following Semantic Versioning).