test(e2e): delete single and multiple SBOMs - #1182
Open
vobratil wants to merge 1 commit into
Open
Conversation
Contributor
Reviewer's GuideAdds new Playwright-based E2E API tests for SBOM deletion, covering single vs multiple and existing vs non-existent SBOM IDs, with shared upload/cleanup helpers and logging for non-expected statuses. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- Remove
test.describe.onlyfrom the suite to avoid unintentionally skipping other e2e tests when this file is run as part of the full test suite. - Consider extracting the random SBOM ID generation logic into a shared helper to avoid duplication between the single and multiple non-existent SBOM tests and keep the test code easier to maintain.
- Since
sbomIdsDeleteis used for cleanup but those SBOMs are already deleted in the tests, it may be clearer either to skip adding already-deleted IDs to this array or to document thatdeleteSbomstolerates missing resources to avoid confusion for future readers.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Remove `test.describe.only` from the suite to avoid unintentionally skipping other e2e tests when this file is run as part of the full test suite.
- Consider extracting the random SBOM ID generation logic into a shared helper to avoid duplication between the single and multiple non-existent SBOM tests and keep the test code easier to maintain.
- Since `sbomIdsDelete` is used for cleanup but those SBOMs are already deleted in the tests, it may be clearer either to skip adding already-deleted IDs to this array or to document that `deleteSboms` tolerates missing resources to avoid confusion for future readers.
## Individual Comments
### Comment 1
<location path="e2e/tests/api/features/sbom-delete.ts" line_range="24" />
<code_context>
+ "cnv-4.17-binary-2-latest.json.bz2",
+];
+
+test.describe.only("SBOM / Delete", () => {
+ test.describe.configure({ mode: "serial" });
+
</code_context>
<issue_to_address>
**issue (testing):** Avoid using `describe.only` in committed e2e tests
`test.describe.only` limits the run to this suite and will skip other e2e suites, potentially hiding regressions. Please remove `.only` (or wrap it in a local-only helper) so the full e2e suite runs in CI.
</issue_to_address>
### Comment 2
<location path="e2e/tests/api/features/sbom-delete.ts" line_range="84-100" />
<code_context>
+ }
+ });
+
+ test("Single non-existent SBOM", async ({ axios }) => {
+ const nonExistentId = Array.from({ length: 45 }, () =>
+ "abcdefghijklmnopqrstuvwxyz0123456789"[Math.floor(Math.random() * 36)],
+ ).join("");
+
+ const response = await axios.delete(
+ `/api/v3/sbom/${encodeURIComponent(nonExistentId)}`,
+ { validateStatus: null },
+ );
+ if (response.status !== 404) {
+ logger.error(
+ `Delete single non-existent SBOM failed with status ${response.status}:`,
+ response.data,
+ );
+ }
+ expect(response.status).toBe(404);
+ });
+
</code_context>
<issue_to_address>
**suggestion (testing):** Avoid randomness for non-existent IDs to keep tests deterministic and reproducible
Using `Math.random` here makes failures hard to reproduce and, in rare cases, could even generate a valid ID. In e2e tests it’s preferable to use a fixed clearly invalid ID pattern (e.g. `"nonexistent-sbom-id-..."`) or a seeded deterministic generator so runs remain predictable while still exercising the "non-existent" path.
```suggestion
test("Single non-existent SBOM", async ({ axios }) => {
// Use a fixed clearly invalid ID to keep the test deterministic and reproducible
const nonExistentId = "nonexistent-sbom-id-should-return-404";
const response = await axios.delete(
`/api/v3/sbom/${encodeURIComponent(nonExistentId)}`,
{ validateStatus: null },
);
if (response.status !== 404) {
logger.error(
`Delete single non-existent SBOM failed with status ${response.status}:`,
response.data,
);
}
expect(response.status).toBe(404);
});
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| "cnv-4.17-binary-2-latest.json.bz2", | ||
| ]; | ||
|
|
||
| test.describe.only("SBOM / Delete", () => { |
Contributor
There was a problem hiding this comment.
issue (testing): Avoid using describe.only in committed e2e tests
test.describe.only limits the run to this suite and will skip other e2e suites, potentially hiding regressions. Please remove .only (or wrap it in a local-only helper) so the full e2e suite runs in CI.
Comment on lines
+84
to
+100
| test("Single non-existent SBOM", async ({ axios }) => { | ||
| const nonExistentId = Array.from({ length: 45 }, () => | ||
| "abcdefghijklmnopqrstuvwxyz0123456789"[Math.floor(Math.random() * 36)], | ||
| ).join(""); | ||
|
|
||
| const response = await axios.delete( | ||
| `/api/v3/sbom/${encodeURIComponent(nonExistentId)}`, | ||
| { validateStatus: null }, | ||
| ); | ||
| if (response.status !== 404) { | ||
| logger.error( | ||
| `Delete single non-existent SBOM failed with status ${response.status}:`, | ||
| response.data, | ||
| ); | ||
| } | ||
| expect(response.status).toBe(404); | ||
| }); |
Contributor
There was a problem hiding this comment.
suggestion (testing): Avoid randomness for non-existent IDs to keep tests deterministic and reproducible
Using Math.random here makes failures hard to reproduce and, in rare cases, could even generate a valid ID. In e2e tests it’s preferable to use a fixed clearly invalid ID pattern (e.g. "nonexistent-sbom-id-...") or a seeded deterministic generator so runs remain predictable while still exercising the "non-existent" path.
Suggested change
| test("Single non-existent SBOM", async ({ axios }) => { | |
| const nonExistentId = Array.from({ length: 45 }, () => | |
| "abcdefghijklmnopqrstuvwxyz0123456789"[Math.floor(Math.random() * 36)], | |
| ).join(""); | |
| const response = await axios.delete( | |
| `/api/v3/sbom/${encodeURIComponent(nonExistentId)}`, | |
| { validateStatus: null }, | |
| ); | |
| if (response.status !== 404) { | |
| logger.error( | |
| `Delete single non-existent SBOM failed with status ${response.status}:`, | |
| response.data, | |
| ); | |
| } | |
| expect(response.status).toBe(404); | |
| }); | |
| test("Single non-existent SBOM", async ({ axios }) => { | |
| // Use a fixed clearly invalid ID to keep the test deterministic and reproducible | |
| const nonExistentId = "nonexistent-sbom-id-should-return-404"; | |
| const response = await axios.delete( | |
| `/api/v3/sbom/${encodeURIComponent(nonExistentId)}`, | |
| { validateStatus: null }, | |
| ); | |
| if (response.status !== 404) { | |
| logger.error( | |
| `Delete single non-existent SBOM failed with status ${response.status}:`, | |
| response.data, | |
| ); | |
| } | |
| expect(response.status).toBe(404); | |
| }); |
vobratil
force-pushed
the
api-delete-tests
branch
from
July 27, 2026 12:38
16823d8 to
0d448da
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1182 +/- ##
=======================================
Coverage 54.03% 54.03%
=======================================
Files 269 269
Lines 5904 5904
Branches 1849 1849
=======================================
Hits 3190 3190
Misses 2419 2419
Partials 295 295
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
vobratil
force-pushed
the
api-delete-tests
branch
from
July 27, 2026 12:44
0d448da to
8931473
Compare
Signed-off-by: Vilem Obratil <vobratil@redhat.com>
vobratil
force-pushed
the
api-delete-tests
branch
from
July 27, 2026 12:53
8931473 to
7d36f08
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
This is a set of simple delete tests that test the deletion of single, multiple existent and non-existent SBOMs.
This should probably not be merged before this issue is fixed:
https://redhat.atlassian.net/browse/TC-5382
Summary by Sourcery
Add end-to-end API coverage for deleting SBOMs under various scenarios.
Tests: