test(sbom): add SPDX 2.3 schema validation with spdx-tools#1040
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (3)
📝 WalkthroughWalkthroughAdds Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/test_sbom.py`:
- Around line 15-22: The _validate_spdx helper currently creates a temporary
file with delete=False and never removes it; change it to record the temp file
name, exit the NamedTemporaryFile context, then in a try/finally call
parse_file(filename) and validate_full_spdx_document(parsed,
spdx_version="SPDX-2.3") and run the assert, and in the finally block remove the
temp file (os.remove(filename)); ensure os is imported and reference
_validate_spdx, parse_file, and validate_full_spdx_document when making the
change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 90cde631-df9f-4d98-8f6f-035307bf98e4
📒 Files selected for processing (2)
pyproject.tomltests/test_sbom.py
mprpic
force-pushed
the
spdx-schema-validation-in-tests
branch
from
d311502 to
a3aca59
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/test_sbom.py (1)
174-194: Consider adding_validate_spdxto the generated SBOM in this test.
test_write_sbom_preserves_existing_filesgenerates an SBOM viasbom.generate_sbom()but doesn't validate it, unlike all other tests. If intentional (since focus is file preservation), ignore. Otherwise:sbom.write_sbom(sbom=doc, dist_info_dir=dist_info_dir) + _validate_spdx(doc) # Existing file should be untouched🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_sbom.py` around lines 174 - 194, The test test_write_sbom_preserves_existing_files creates an SBOM via sbom.generate_sbom() but never runs the SPDX validator; add a call to _validate_spdx on the generated document before writing to ensure parity with other tests. Locate the test function test_write_sbom_preserves_existing_files and after doc = sbom.generate_sbom(...), invoke _validate_spdx(doc) (or the module’s public validate wrapper if present) so the generated SBOM is validated prior to calling sbom.write_sbom; keep the existing assertions about file preservation unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/test_sbom.py`:
- Around line 174-194: The test test_write_sbom_preserves_existing_files creates
an SBOM via sbom.generate_sbom() but never runs the SPDX validator; add a call
to _validate_spdx on the generated document before writing to ensure parity with
other tests. Locate the test function test_write_sbom_preserves_existing_files
and after doc = sbom.generate_sbom(...), invoke _validate_spdx(doc) (or the
module’s public validate wrapper if present) so the generated SBOM is validated
prior to calling sbom.write_sbom; keep the existing assertions about file
preservation unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 32e17e2c-32f1-4010-b124-e2c92a845071
📒 Files selected for processing (2)
pyproject.tomltests/test_sbom.py
mprpic
force-pushed
the
spdx-schema-validation-in-tests
branch
from
a3aca59 to
8aafb11
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/test_sbom.py (1)
175-195:⚠️ Potential issue | 🟡 MinorAdd SPDX schema validation in
test_write_sbom_preserves_existing_files.This test verifies file preservation/existence but not that the newly written SPDX file is schema-valid, leaving one SBOM path unvalidated.
Proposed fix
def test_write_sbom_preserves_existing_files(tmp_path: pathlib.Path) -> None: @@ - sbom.write_sbom(sbom=doc, dist_info_dir=dist_info_dir) + sbom.write_sbom(sbom=doc, dist_info_dir=dist_info_dir) + generated = sboms_dir / "fromager.spdx.json" @@ - assert (sboms_dir / "fromager.spdx.json").exists() + assert generated.exists() + _validate_spdx(json.loads(generated.read_text()))As per coding guidelines
tests/**: Verify test actually tests the intended behavior. Check for missing edge cases.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_sbom.py` around lines 175 - 195, The test currently checks file existence but not that the newly written SPDX SBOM is schema-valid; after calling sbom.write_sbom in test_write_sbom_preserves_existing_files, load the JSON from (sboms_dir / "fromager.spdx.json"), parse it with json.loads, and validate it using jsonschema.validate(instance, schema) against the SPDX JSON schema (obtain the SPDX JSON schema into a variable such as spdx_schema from your test fixtures or the SPDX project's schema) so the test asserts the created SPDX file is schema-compliant in addition to existing-file preservation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@tests/test_sbom.py`:
- Around line 175-195: The test currently checks file existence but not that the
newly written SPDX SBOM is schema-valid; after calling sbom.write_sbom in
test_write_sbom_preserves_existing_files, load the JSON from (sboms_dir /
"fromager.spdx.json"), parse it with json.loads, and validate it using
jsonschema.validate(instance, schema) against the SPDX JSON schema (obtain the
SPDX JSON schema into a variable such as spdx_schema from your test fixtures or
the SPDX project's schema) so the test asserts the created SPDX file is
schema-compliant in addition to existing-file preservation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e89b1b69-27c3-4164-aebf-64b32bd1031b
📒 Files selected for processing (3)
AGENTS.mdpyproject.tomltests/test_sbom.py
✅ Files skipped from review due to trivial changes (1)
- AGENTS.md
🚧 Files skipped from review as they are similar to previous changes (1)
- pyproject.toml
|
@mergify rebase |
Validate generated SBOMs against the SPDX 2.3 spec in all SBOM tests using the spdx-tools library. Closes: python-wheel-build#1007 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Martin Prpič <mprpic@redhat.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Martin Prpič <mprpic@redhat.com>
|
Deprecation notice: This pull request comes from a fork and was rebased using |
✅ Branch has been successfully rebased |
LalatenduMohanty
force-pushed
the
spdx-schema-validation-in-tests
branch
from
8aafb11 to
6d6aae8
Compare
2 participants
Pull Request Description
What
Validate generated SBOMs against the SPDX 2.3 spec in all SBOM tests using the spdx-tools library.
Why
Ensures that all SBOMs that are generated in tests are valid against the SPDX 2.3 spec. It's enough to do this in tests because doing it during actual builds would slow down the builds even more. (I know from experience the schema validator in this lib is rather slow for very large SBOMs...)