tests: sbom-explorer-details reuse SbomDetailsPage utils #724
Conversation
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
Signed-off-by: Carlos Feria <[email protected]>
Signed-off-by: Carlos Feria <[email protected]>
Reviewer's GuideRefactors SBOM Explorer E2E tests to remove redundant precondition steps and centralize page navigation using SbomDetailsPage utilities, and adds a new SBOM Search test suite covering vulnerability verification and label addition scenarios. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
carlosthe19916
changed the title
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Consider extracting the “An ingested SBOM {string} is available” step into a shared module rather than duplicating it under both sbom-explorer and sbom-search so it can be reused across directories.
- The
User visits SBOM details Page of {string}step assigns to_sbomDetailsPagewithout using it—either consume the returned page object or remove the unused variable to improve clarity. - The xpath-based check in the sbom-search vulnerability step feels brittle; consider adding a helper on
SbomListPageor using a more stable selector to assert non-zero vulnerabilities.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider extracting the “An ingested SBOM {string} is available” step into a shared module rather than duplicating it under both sbom-explorer and sbom-search so it can be reused across directories.
- The `User visits SBOM details Page of {string}` step assigns to `_sbomDetailsPage` without using it—either consume the returned page object or remove the unused variable to improve clarity.
- The xpath-based check in the sbom-search vulnerability step feels brittle; consider adding a helper on `SbomListPage` or using a more stable selector to assert non-zero vulnerabilities.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Signed-off-by: Carlos Feria <[email protected]>
|
Converting it to draft until tests pass in ci |
Signed-off-by: Carlos Feria <[email protected]>
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The sbom-search feature scenarios never navigate to the SBOM search page before applying filters—add an explicit ‘When User visits SBOM search page’ step to ensure tests run in the correct context.
- You have duplicate step definitions for ingesting and checking SBOM vulnerabilities in both @sbom-explorer and @sbom-search; extract these into a shared step module to reduce maintenance overhead.
- Have SbomDetailsPage.build assert a key element (like the page title or an info tab) is visible after navigation so failures are caught early and tests don't silently proceed on the wrong page.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The sbom-search feature scenarios never navigate to the SBOM search page before applying filters—add an explicit ‘When User visits SBOM search page’ step to ensure tests run in the correct context.
- You have duplicate step definitions for ingesting and checking SBOM vulnerabilities in both @sbom-explorer and @sbom-search; extract these into a shared step module to reduce maintenance overhead.
- Have SbomDetailsPage.build assert a key element (like the page title or an info tab) is visible after navigation so failures are caught early and tests don't silently proceed on the wrong page.
## Individual Comments
### Comment 1
<location> `e2e/tests/ui/features/@sbom-search/sbom-search.step.ts:24` </location>
<code_context>
},
);
-Given(
- "An ingested SBOM {string} containing Vulnerabilities",
- async ({ page }, sbomName) => {
- const element = page.locator(
- `xpath=(//tr[contains(.,'${sbomName}')]/td[@data-label='Vulnerabilities']/div)[1]`,
- );
- await expect(element, "SBOM have no vulnerabilities").toHaveText(
- /^(?!0$).+/,
</code_context>
<issue_to_address>
Vulnerability presence check should include zero-vulnerability scenario.
Please add a test case for an SBOM with zero vulnerabilities to verify the UI handles this scenario correctly.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
Given(
"An ingested SBOM {string} containing Vulnerabilities",
async ({ page }, sbomName) => {
const element = page.locator(
`xpath=(//tr[contains(.,'${sbomName}')]/td[@data-label='Vulnerabilities']/div)[1]`,
);
await expect(element, "SBOM have no vulnerabilities").toHaveText(
/^(?!0$).+/,
);
},
);
=======
Given(
"An ingested SBOM {string} containing Vulnerabilities",
async ({ page }, sbomName) => {
const element = page.locator(
`xpath=(//tr[contains(.,'${sbomName}')]/td[@data-label='Vulnerabilities']/div)[1]`,
);
await expect(element, "SBOM have no vulnerabilities").toHaveText(
/^(?!0$).+/,
);
},
);
Given(
"An ingested SBOM {string} containing zero Vulnerabilities",
async ({ page }, sbomName) => {
const element = page.locator(
`xpath=(//tr[contains(.,'${sbomName}')]/td[@data-label='Vulnerabilities']/div)[1]`,
);
await expect(element, "SBOM should show zero vulnerabilities").toHaveText(
/^0$/,
);
},
);
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
# Conflicts: # e2e/tests/ui/features/@sbom-explorer/sbom-explorer.feature # e2e/tests/ui/features/@sbom-explorer/sbom-explorer.step.ts
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #724 +/- ##
==========================================
- Coverage 62.03% 61.05% -0.98%
==========================================
Files 156 156
Lines 2655 2655
Branches 601 601
==========================================
- Hits 1647 1621 -26
- Misses 778 802 +24
- Partials 230 232 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Depends on #723
This PR includes all changes from #723 so that PR needs to be merged first.
Given An ingested SBOM "<sbomName>" is availableand let the stepUser visits SBOM details Page of "<sbomName>"do all the navigation.Reasoning:
All tests within the @sbom-explorer directory should assume we successfully arrived to SBOM Details Page; therefore we can safely use our utility classes for navigating to the SBOM Details Page.
Summary by Sourcery
Refactor sbom-explorer end-to-end tests to reuse SbomDetailsPage utilities for navigation and remove redundant Given steps; extract shared SBOM list page steps into a new sbom-search test suite for vulnerability checks and label management.
New Features:
Enhancements:
Tests: