chore(test): adjust order of vulnerabilities after a backend change - #1205
Merged
Conversation
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates an end-to-end UI test’s expected vulnerability ordering to match the backend’s new sort order, resolving failing CI assertions. 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 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="e2e/tests/ui/pages/home/attention.spec.ts" line_range="8" />
<code_context>
import { HomePage } from "./HomePage";
const EXPECTED_TOP_3 = [
- { identifier: "CVE-2026-18236", severity: /critical/i },
{ identifier: "CVE-2026-63223", severity: /critical/i },
+ { identifier: "CVE-2026-18236", severity: /critical/i },
{ identifier: "CVE-2026-18358", severity: /high/i },
] as const;
</code_context>
<issue_to_address>
**suggestion (testing):** Consider making the test resilient to ordering changes when vulnerabilities share the same severity.
The test currently depends on the exact order of the two critical vulnerabilities. Since the backend has already changed this once, it would be more robust to assert that both critical CVEs appear in the "top" vulnerabilities section (and are marked as critical) without relying on their relative order—unless the product explicitly guarantees a specific sort criterion. If ordering is guaranteed, please add a brief comment in the test documenting the expected sort key so future backend changes don’t unintentionally break CI.
Suggested implementation:
```typescript
import { HomePage } from "./HomePage";
/**
* Expected vulnerabilities in the "Top vulnerabilities" section.
*
* Do not rely on the relative ordering of vulnerabilities that share
* the same severity (e.g. multiple critical CVEs): backend sorting may
* change as long as the section still contains the same set of items.
*/
const EXPECTED_TOP_3 = [
{ identifier: "CVE-2026-63223", severity: /critical/i },
{ identifier: "CVE-2026-18236", severity: /critical/i },
{ identifier: "CVE-2026-18358", severity: /high/i },
] as const;
/**
* Subset of EXPECTED_TOP_3 that should be marked as critical.
* Tests should use this to assert presence and severity of critical CVEs
* without depending on their order in the list.
*/
const EXPECTED_CRITICAL_TOP_VULNERABILITIES = EXPECTED_TOP_3.filter(
({ severity }) => severity.test("critical"),
);
```
To fully implement the suggestion and make the test resilient to ordering changes:
1. Update the assertion that checks the "Top vulnerabilities" list so that it does **not** assert exact ordering for the critical CVEs.
- Instead of something like `expect(renderedTopVulns).toEqual(EXPECTED_TOP_3)`, use a combination of:
- `expect(renderedTopVulns).toEqual(expect.arrayContaining(EXPECTED_TOP_3));` if you only care about membership, or
- `expect(renderedTopVulns.map(v => v.identifier)).toEqual(expect.arrayContaining(EXPECTED_CRITICAL_TOP_VULNERABILITIES.map(v => v.identifier)));`
plus explicit severity checks per item.
2. Ensure the test verifies that:
- All identifiers from `EXPECTED_CRITICAL_TOP_VULNERABILITIES` appear in the "top vulnerabilities" section.
- Each of those entries is marked as critical (matching the `/critical/i` severity).
- The test does not assume any specific relative order between the critical CVEs.
3. If the product *does* guarantee a specific sort key (e.g. `severity desc, exploitability desc, identifier asc`), add a short inline comment near the assertion documenting that sort criterion so backend changes do not unintentionally break CI.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| import { HomePage } from "./HomePage"; | ||
|
|
||
| const EXPECTED_TOP_3 = [ | ||
| { identifier: "CVE-2026-18236", severity: /critical/i }, |
Contributor
There was a problem hiding this comment.
suggestion (testing): Consider making the test resilient to ordering changes when vulnerabilities share the same severity.
The test currently depends on the exact order of the two critical vulnerabilities. Since the backend has already changed this once, it would be more robust to assert that both critical CVEs appear in the "top" vulnerabilities section (and are marked as critical) without relying on their relative order—unless the product explicitly guarantees a specific sort criterion. If ordering is guaranteed, please add a brief comment in the test documenting the expected sort key so future backend changes don’t unintentionally break CI.
Suggested implementation:
import { HomePage } from "./HomePage";
/**
* Expected vulnerabilities in the "Top vulnerabilities" section.
*
* Do not rely on the relative ordering of vulnerabilities that share
* the same severity (e.g. multiple critical CVEs): backend sorting may
* change as long as the section still contains the same set of items.
*/
const EXPECTED_TOP_3 = [
{ identifier: "CVE-2026-63223", severity: /critical/i },
{ identifier: "CVE-2026-18236", severity: /critical/i },
{ identifier: "CVE-2026-18358", severity: /high/i },
] as const;
/**
* Subset of EXPECTED_TOP_3 that should be marked as critical.
* Tests should use this to assert presence and severity of critical CVEs
* without depending on their order in the list.
*/
const EXPECTED_CRITICAL_TOP_VULNERABILITIES = EXPECTED_TOP_3.filter(
({ severity }) => severity.test("critical"),
);To fully implement the suggestion and make the test resilient to ordering changes:
- Update the assertion that checks the "Top vulnerabilities" list so that it does not assert exact ordering for the critical CVEs.
- Instead of something like
expect(renderedTopVulns).toEqual(EXPECTED_TOP_3), use a combination of:expect(renderedTopVulns).toEqual(expect.arrayContaining(EXPECTED_TOP_3));if you only care about membership, orexpect(renderedTopVulns.map(v => v.identifier)).toEqual(expect.arrayContaining(EXPECTED_CRITICAL_TOP_VULNERABILITIES.map(v => v.identifier)));
plus explicit severity checks per item.
- Instead of something like
- Ensure the test verifies that:
- All identifiers from
EXPECTED_CRITICAL_TOP_VULNERABILITIESappear in the "top vulnerabilities" section. - Each of those entries is marked as critical (matching the
/critical/iseverity). - The test does not assume any specific relative order between the critical CVEs.
- All identifiers from
- If the product does guarantee a specific sort key (e.g.
severity desc, exploitability desc, identifier asc), add a short inline comment near the assertion documenting that sort criterion so backend changes do not unintentionally break CI.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1205 +/- ##
==========================================
- Coverage 54.20% 54.19% -0.02%
==========================================
Files 255 255
Lines 5715 5715
Branches 1774 1774
==========================================
- Hits 3098 3097 -1
Misses 2355 2355
- Partials 262 263 +1
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:
|
mrrajan
reviewed
Aug 13, 2026
mrrajan
left a comment
mrrajan
left a comment
Contributor
There was a problem hiding this comment.
@matejnesuta Thanks for raising this PR, But this issue will be resolved with backend fix guacsec/trustify#2569
mrrajan
approved these changes
Aug 13, 2026
mrrajan
left a comment
mrrajan
left a comment
Contributor
There was a problem hiding this comment.
LGTM, Even though the backend fix required for CVSS scoring - this fix also essential for correct ordering.
Contributor
|
Successfully created backport PR for |
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
2 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.
Summary
Changed the order of vulnerabilities in a test to match to address a backend fix. Hopefully this fixes current issues with the CI.
Related Issues
guacsec/trustify#2569
Summary by Sourcery
Tests: