RHAIENG-304: tests(pytests): add check for the pylock.toml integrity with respect to pyproject.toml #2233
There are no files selected for viewing
This file contains hidden or 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 |
|---|---|---|
|
|
@@ -7,11 +7,13 @@ | |
| import tomllib | ||
| from typing import TYPE_CHECKING | ||
|
|
||
| import packaging.requirements | ||
| import pytest | ||
|
|
||
| from tests import PROJECT_ROOT | ||
|
|
||
| if TYPE_CHECKING: | ||
| from typing import Any | ||
| import pytest_subtests | ||
|
|
||
| MAKE = shutil.which("gmake") or shutil.which("make") | ||
|
|
@@ -27,15 +29,31 @@ def test_image_pyprojects(subtests: pytest_subtests.plugin.SubTests): | |
| except ValueError: | ||
| pytest.skip(f"skipping {directory.name}/pyproject.toml as it is not an image directory") | ||
|
|
||
| pyproject = tomllib.loads(file.read_text()) | ||
| with subtests.test(msg="checking pyproject.toml", pyproject=file): | ||
| assert "project" in pyproject, "pyproject.toml is missing a [project] section" | ||
| assert "requires-python" in pyproject["project"], ( | ||
| "pyproject.toml is missing a [project.requires-python] section" | ||
| ) | ||
| assert pyproject["project"]["requires-python"] == f"=={python}.*", ( | ||
| "pyproject.toml does not declare the expected Python version" | ||
| ) | ||
|
|
||
| assert "dependencies" in pyproject["project"], ( | ||
| "pyproject.toml is missing a [project.dependencies] section" | ||
| ) | ||
|
|
||
| pylock = tomllib.loads(file.with_name("pylock.toml").read_text()) | ||
| pylock_packages: dict[str, dict[str, Any]] = {p["name"]: p for p in pylock["packages"]} | ||
| with subtests.test(msg="checking pylock.toml consistency with pyproject.toml", pyproject=file): | ||
| for d in pyproject["project"]["dependencies"]: | ||
| requirement = packaging.requirements.Requirement(d) | ||
|
|
||
| assert requirement.name in pylock_packages, f"Dependency {d} is not in pylock.toml" | ||
| version = pylock_packages[requirement.name]["version"] | ||
| assert requirement.specifier.contains(version), ( | ||
| f"Version of {d} in pyproject.toml does not match {version=} in pylock.toml" | ||
| ) | ||
|
Comment on lines
+46
to
+56
There was a problem hiding this comment. 🛠️ Refactor suggestion Handle name normalization, markers, and non-PEP440 (VCS/URL) locks.
Apply this diff: - pylock = tomllib.loads(file.with_name("pylock.toml").read_text())
- pylock_packages: dict[str, dict[str, Any]] = {p["name"]: p for p in pylock["packages"]}
+ pylock = tomllib.load(file.with_name("pylock.toml").open("rb"))
+ assert "packages" in pylock, "pylock.toml missing [packages] list"
+ pylock_packages: dict[str, dict[str, Any]] = {
+ canonicalize_name(p["name"]): p for p in pylock["packages"]
+ }
with subtests.test(msg="checking pylock.toml consistency with pyproject.toml", pyproject=file):
- for d in pyproject["project"]["dependencies"]:
- requirement = packaging.requirements.Requirement(d)
-
- assert requirement.name in pylock_packages, f"Dependency {d} is not in pylock.toml"
- version = pylock_packages[requirement.name]["version"]
- assert requirement.specifier.contains(version), (
- f"Version of {d} in pyproject.toml does not match {version=} in pylock.toml"
- )
+ # Evaluate markers against the image environment (Linux) and parsed Python version.
+ _env = {"python_version": python, "sys_platform": "linux", "platform_system": "Linux"}
+ for d in pyproject["project"]["dependencies"]:
+ requirement = Requirement(d)
+ if requirement.marker and not requirement.marker.evaluate(_env):
+ continue # not applicable to this image
+ name = canonicalize_name(requirement.name)
+ assert name in pylock_packages, f"Dependency {d} is not in pylock.toml"
+ locked = pylock_packages[name]
+ locked_version = locked.get("version")
+ # Only assert specifier if we have a PEP 440 version (skip VCS/URL-only pins)
+ if locked_version is not None:
+ assert requirement.specifier.contains(locked_version), (
+ f"{d} does not satisfy locked version {locked_version} in pylock.toml"
+ )
There was a problem hiding this comment. this does not seem to be necessary for now, the test is passing just fine @coderabbitai please remember this suggestion for later, if I ever encounter problems with the current code and may need to implement it this way There was a problem hiding this comment. @jiridanek, understood! I've noted that the current implementation is working well for your use case. I'll remember the suggestion for future reference in case you encounter edge cases that require the more robust handling. ✏️ Learnings added
🧠 Learnings used |
||
|
|
||
|
|
||
| def test_files_that_should_be_same_are_same(subtests: pytest_subtests.plugin.SubTests): | ||
|
|
||
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.
Uh oh!
There was an error while loading. Please reload this page.
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.
🛠️ Refactor suggestion
Canonicalize names and import directly to avoid false negatives.
Lock/package name casing and hyphen/underscore differences can cause mismatches (e.g., MarkupSafe vs markupsafe). Canonicalize both sides and switch to direct imports to simplify call sites.
Apply this diff:
📝 Committable suggestion
🤖 Prompt for AI Agents
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'll consider this for subsequent pr, thanks; will be checking if imagestream yamls also correspond to the lock file, it will be useful there, both SpecifierSet and canonicalize_name
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.
@jiridanek, understood! That makes perfect sense to defer this for a subsequent PR, especially since you'll be extending the validation to imagestream yamls as well. The canonicalize_name and SpecifierSet utilities will indeed be very helpful for ensuring consistent package name matching across different file formats and naming conventions.
✏️ Learnings added
🧠 Learnings used