Added API tests for the Latest SBOM Analysis endpoint.#949
Open
vobratil wants to merge 2 commits intoguacsec:mainfrom
Open
Added API tests for the Latest SBOM Analysis endpoint.#949vobratil wants to merge 2 commits intoguacsec:mainfrom
vobratil wants to merge 2 commits intoguacsec:mainfrom
Conversation
Contributor
Reviewer's GuideAdds comprehensive end-to-end API tests for the File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #949 +/- ##
==========================================
+ Coverage 61.76% 61.79% +0.02%
==========================================
Files 207 207
Lines 3688 3688
Branches 836 836
==========================================
+ Hits 2278 2279 +1
+ Misses 1101 1099 -2
- Partials 309 310 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
Author
|
I've just noticed an issue with the ingestion and cleanup of SPDX SBOMs. I'll have to make a few quick changes. |
Signed-off-by: Vilem Obratil <vobratil@redhat.com>
Contributor
Author
|
Okay, we should be back in business. |
![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:
- The new test files repeat very similar deeply nested
expect.objectContaining/arrayContainingstructures across many cases; consider extracting shared assertion helpers (e.g.,expectCdxPublishedDates(...),expectSpdxPublishedDates(...)) to reduce duplication and make the intent clearer. - In the test suites you declare mutable arrays with
varfor SBOM IDs (e.g.,var sbomIdsLatestBasicOlder: string[] = []); switching to block-scopedletorconstwould avoid potential scoping surprises and better reflect how they are used.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new test files repeat very similar deeply nested `expect.objectContaining`/`arrayContaining` structures across many cases; consider extracting shared assertion helpers (e.g., `expectCdxPublishedDates(...)`, `expectSpdxPublishedDates(...)`) to reduce duplication and make the intent clearer.
- In the test suites you declare mutable arrays with `var` for SBOM IDs (e.g., `var sbomIdsLatestBasicOlder: string[] = []`); switching to block-scoped `let` or `const` would avoid potential scoping surprises and better reflect how they are used.
## Individual Comments
### Comment 1
<location path="e2e/tests/api/features/analysis-latest-basic.ts" line_range="67" />
<code_context>
+ });
+
+ test.describe("CDX", () => {
+ test.describe("Older SBOM", () => {
+ test.beforeAll(async ({ axios }) => {
+ const fullSbomPaths = getFullSbomPaths(sbomDir, sbomsCdxOlder);
</code_context>
<issue_to_address>
**issue (testing):** Missing teardown for SPDX "Older SBOM" uploads, which can leave test data behind and influence other tests
In this SPDX block you collect `sbomIdsLatestBasicOlderSpdx` in `beforeAll` but never clean them up (unlike the CDX older SBOMs and SPDX latest SBOMs, which call `deleteSboms`). This can leave extra SBOMs in the system and interfere with later tests. Add an `afterAll` in this "Older SBOM" describe that calls `await deleteSboms(axios, sbomIdsLatestBasicOlderSpdx);` so the fixture remains isolated and repeatable.
</issue_to_address>
### Comment 2
<location path="e2e/tests/api/features/analysis-latest-issues.ts" line_range="39-30" />
<code_context>
+ await deleteSboms(axios, sbomIdsTopLevelAncestorIsUpstream);
+ });
+
+ test(
+ "Get product by CPE",
+ {
+ annotation: {
+ type: "issue",
+ description: "https://issues.redhat.com/browse/TC-3624",
+ },
+ },
+ async ({ axios }) => {
+ const urlEncodedComponentCpe = encodeURIComponent(
+ topLevelAncestorIsUpstreamProductCpe,
+ );
+
+ const response = await axios.get(
</code_context>
<issue_to_address>
**suggestion (testing):** Strengthen TC-3278 coverage by constraining the full result set, not just a few expected products
In the "Creator is 'Red Hat' / Get component by pURL" test, you only check that one unwanted product is absent and two expected products are present. The test would still pass if extra, unrelated products were included. To make it more robust, assert either the exact set of product names returned (e.g., `items.map(i => i.product_name)` equals `[4.13, 4.12]` in any order) or that every returned item’s `product_name` is in an allowed set.
Suggested implementation:
```typescript
test(
"Creator is 'Red Hat' / Get component by pURL",
{
annotation: {
type: "issue",
description: "https://issues.redhat.com/browse/TC-3278",
},
},
async ({ axios }) => {
const encodedPurl = encodeURIComponent(creatorIsRedHatComponentPurl);
const response = await axios.get(
`/api/v2/analysis/latest/component/${encodedPurl}?creator=Red%20Hat`,
);
const items = response.data.items ?? [];
const productNames = items.map((item: { product_name?: string }) => item.product_name);
const allowedProductNames = [
"OpenShift Container Platform 4.12",
"OpenShift Container Platform 4.13",
];
// Ensure we got exactly the expected set of product names (no more, no less)
expect(productNames).toHaveLength(allowedProductNames.length);
expect(productNames).toEqual(expect.arrayContaining(allowedProductNames));
// Extra safety: every returned product_name must be in the allowed set
const allowedSet = new Set(allowedProductNames);
for (const name of productNames) {
expect(allowedSet.has(name)).toBe(true);
}
},
);
```
If the exact product names or the excluded product (`4.11`) differ in your codebase, update `allowedProductNames` to match the real expected values.
If your Jest/Playwright setup uses a stricter `items` type, you may want to replace the inline type `{ product_name?: string }` with the appropriate interface/type already defined in the tests.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| }); | ||
|
|
||
| test.describe("CDX", () => { | ||
| test.describe("Older SBOM", () => { |
Contributor
There was a problem hiding this comment.
issue (testing): Missing teardown for SPDX "Older SBOM" uploads, which can leave test data behind and influence other tests
In this SPDX block you collect sbomIdsLatestBasicOlderSpdx in beforeAll but never clean them up (unlike the CDX older SBOMs and SPDX latest SBOMs, which call deleteSboms). This can leave extra SBOMs in the system and interfere with later tests. Add an afterAll in this "Older SBOM" describe that calls await deleteSboms(axios, sbomIdsLatestBasicOlderSpdx); so the fixture remains isolated and repeatable.
| const fullSbomPaths = getFullSbomPaths( | ||
| sbomDir, | ||
| topLevelAncestorIsUpstreamSboms, | ||
| ); |
Contributor
There was a problem hiding this comment.
suggestion (testing): Strengthen TC-3278 coverage by constraining the full result set, not just a few expected products
In the "Creator is 'Red Hat' / Get component by pURL" test, you only check that one unwanted product is absent and two expected products are present. The test would still pass if extra, unrelated products were included. To make it more robust, assert either the exact set of product names returned (e.g., items.map(i => i.product_name) equals [4.13, 4.12] in any order) or that every returned item’s product_name is in an allowed set.
Suggested implementation:
test(
"Creator is 'Red Hat' / Get component by pURL",
{
annotation: {
type: "issue",
description: "https://issues.redhat.com/browse/TC-3278",
},
},
async ({ axios }) => {
const encodedPurl = encodeURIComponent(creatorIsRedHatComponentPurl);
const response = await axios.get(
`/api/v2/analysis/latest/component/${encodedPurl}?creator=Red%20Hat`,
);
const items = response.data.items ?? [];
const productNames = items.map((item: { product_name?: string }) => item.product_name);
const allowedProductNames = [
"OpenShift Container Platform 4.12",
"OpenShift Container Platform 4.13",
];
// Ensure we got exactly the expected set of product names (no more, no less)
expect(productNames).toHaveLength(allowedProductNames.length);
expect(productNames).toEqual(expect.arrayContaining(allowedProductNames));
// Extra safety: every returned product_name must be in the allowed set
const allowedSet = new Set(allowedProductNames);
for (const name of productNames) {
expect(allowedSet.has(name)).toBe(true);
}
},
);If the exact product names or the excluded product (4.11) differ in your codebase, update allowedProductNames to match the real expected values.
If your Jest/Playwright setup uses a stricter items type, you may want to replace the inline type { product_name?: string } with the appropriate interface/type already defined in the tests.
Signed-off-by: Vilem Obratil <vobratil@redhat.com>
matejnesuta
requested changes
Mar 19, 2026
Contributor
matejnesuta
left a comment
matejnesuta
left a comment
There was a problem hiding this comment.
Just a small suggestion.
| `/api/v2/analysis/latest/component/${urlEncodedComponentCpe}?descendants=100`, | ||
| ); | ||
|
|
||
| expect(response.data.items).toEqual( |
Contributor
There was a problem hiding this comment.
I am not that familiar with the test cases here, but I feel like this assertion could be generalized somehow.
| `/api/v2/analysis/latest/component?ancestors=100&q=purl~${urlEncodedComponentPurl}&limit=100&offset=0`, | ||
| ); | ||
|
|
||
| expect(response.data.items).toEqual( |
Contributor
There was a problem hiding this comment.
Same as my previous comment.
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
4 participants
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 pull request adds a set of API tests for the Latest SBOM Analysis endpoint and refactors the
uploadFilesmethod, so that it can be reused in tests.Summary by Sourcery
Add end-to-end API test coverage for the Latest SBOM Analysis endpoint and introduce reusable helpers for uploading and cleaning up SBOM test data.
New Features:
Enhancements: