test(e2e): add SBOM Groups BDD test coverage#1108
Draft
mrrajan wants to merge 9 commits into
Draft
Conversation
Contributor
Reviewer's GuideAdds comprehensive BDD test coverage for SBOM Groups UI, including product labeling, hierarchical tree behavior, parent-child relationships, invalid group navigation, SBOM counts, breadcrumbs, and sorting, plus minor cleanup of existing step definitions. 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 #1108 +/- ##
==========================================
+ Coverage 53.07% 53.09% +0.02%
==========================================
Files 270 270
Lines 5879 5937 +58
Branches 1842 1870 +28
==========================================
+ Hits 3120 3152 +32
- Misses 2455 2482 +27
+ Partials 304 303 -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:
|
![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:
- There is repeated setup logic for ensuring groups (and parent/child relationships) exist across multiple steps; consider extracting this into shared helper utilities to reduce duplication and keep test maintenance easier.
- The invalid group ID error assertion uses a broad heading regex (
/error|not found|something went wrong/i); tightening this to a known error message or a more specific selector would make the test less flaky and more resilient to copy changes. - Several selectors rely on visible text patterns (e.g.,
text=/\d+ SBOMs?/and label text like "Product"); if UI copy changes are expected, you may want to pivot to more stable attributes or test-ids to avoid brittle tests.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There is repeated setup logic for ensuring groups (and parent/child relationships) exist across multiple steps; consider extracting this into shared helper utilities to reduce duplication and keep test maintenance easier.
- The invalid group ID error assertion uses a broad heading regex (`/error|not found|something went wrong/i`); tightening this to a known error message or a more specific selector would make the test less flaky and more resilient to copy changes.
- Several selectors rely on visible text patterns (e.g., `text=/\d+ SBOMs?/` and label text like "Product"); if UI copy changes are expected, you may want to pivot to more stable attributes or test-ids to avoid brittle tests.
## Individual Comments
### Comment 1
<location path="e2e/tests/ui/features/@sbom-groups/sbom-groups.step.ts" line_range="598-602" />
<code_context>
+ await page.goto("/sbom-groups/invalid-group-id-12345");
+});
+
+Then("An error state is displayed for the invalid group", async ({ page }) => {
+ const errorHeading = page.getByRole("heading", {
+ name: /error|not found|something went wrong/i,
+ });
+ await expect(errorHeading).toBeVisible({ timeout: 10000 });
+});
+
</code_context>
<issue_to_address>
**suggestion (testing):** Error state assertion for invalid group ID is quite generic and may produce false positives
This assertion currently matches any heading with `/error|not found|something went wrong/i`, which could pick up unrelated page errors and cause false positives. To make the test reliably about SBOM group errors, assert against a more specific selector or message for the SBOM groups error UI (e.g., a dedicated data-testid or fixed heading text), and optionally check that the URL stays on the invalid group route. That way the test proves the invalid group ID triggers the intended error state, not just any generic error on the page.
Suggested implementation:
```typescript
// Invalid group ID handling
When("User navigates to group details with invalid ID", async ({ page }) => {
await page.goto("/sbom-groups/invalid-group-id-12345");
});
Then("An error state is displayed for the invalid group", async ({ page }) => {
// Assert we stay on the invalid group route
await expect(page).toHaveURL(/\/sbom-groups\/invalid-group-id-12345$/);
// Assert the dedicated SBOM group error UI is shown
const sbomGroupError = page.getByTestId("sbom-group-error");
await expect(sbomGroupError).toBeVisible();
});
Then("The SBOM Groups table shows all groups", async ({ page }) => {
const table = page.getByRole("treegrid", { name: "sbom-groups-table" });
await expect(table).toBeVisible();
const rows = table.getByRole("row");
await expect(rows.first()).toBeVisible();
});
Then(
```
1. Ensure the SBOM group error UI in the application uses `data-testid="sbom-group-error"` on the container element (e.g., the alert or error panel). If a different test id or fixed heading text is already used (such as `"sbom-group-not-found-heading"` or a specific error message string), update the selector in this step definition to match the existing UI.
2. If the invalid-group route differs (for example, it includes a workspace/org prefix or uses a different path segment), adjust the regular expression in `toHaveURL` to match the actual route shape.
</issue_to_address>
### Comment 2
<location path="e2e/tests/ui/features/@sbom-groups/sbom-groups.step.ts" line_range="445" />
<code_context>
Then(
"The SBOM Groups table shows filtered results containing {string}",
async ({ page }, searchTerm: string) => {
</code_context>
<issue_to_address>
**suggestion (testing):** SBOM count assertion doesn’t distinguish zero vs non-zero counts or missing content
The `Then "The SBOM count is displayed for group {string}"` step only asserts that text matching `"\d+ SBOMs?"` is visible, which passes for `0 SBOMs` and could miss cases where the count is absent but similar text appears. If the behavior requires at least one SBOM, please assert on the parsed numeric value (e.g., `> 0`). Otherwise, add a dedicated scenario for `0 SBOMs` so both zero and non-zero states are explicitly covered.
Suggested implementation:
```typescript
Then(
"The SBOM count is displayed for group {string}",
async ({ page }, groupName: string) => {
const groupRow = page.getByRole("row", { name: new RegExp(groupName, "i") });
await expect(groupRow).toBeVisible();
const sbomCountLocator = groupRow.getByText(/\d+\s+SBOMs?/);
const sbomCountText = await sbomCountLocator.textContent();
expect(sbomCountText).not.toBeNull();
const match = sbomCountText!.match(/(\d+)\s+SBOMs?/);
expect(match).not.toBeNull();
const sbomCount = Number.parseInt(match![1], 10);
expect(sbomCount).toBeGreaterThan(0);
},
);
```
1. Ensure the DOM structure for the SBOM count matches the locator assumptions (count text within the group row and formatted like `"N SBOMs"`). If the count is in a different element (e.g., a specific column), adjust `sbomCountLocator` accordingly (e.g., `groupRow.getByRole("cell", { name: /\d+\s+SBOMs?/ })`).
2. If the product must support a valid `0 SBOMs` state, add a dedicated scenario to your feature file (e.g., `"The SBOM count shows zero SBOMs for group {string}"`) that asserts `sbomCount === 0` instead of `> 0`, using the same parsing logic.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+598
to
+602
| Then("An error state is displayed for the invalid group", async ({ page }) => { | ||
| const errorHeading = page.getByRole("heading", { | ||
| name: /error|not found|something went wrong/i, | ||
| }); | ||
| await expect(errorHeading).toBeVisible({ timeout: 10000 }); |
Contributor
There was a problem hiding this comment.
suggestion (testing): Error state assertion for invalid group ID is quite generic and may produce false positives
This assertion currently matches any heading with /error|not found|something went wrong/i, which could pick up unrelated page errors and cause false positives. To make the test reliably about SBOM group errors, assert against a more specific selector or message for the SBOM groups error UI (e.g., a dedicated data-testid or fixed heading text), and optionally check that the URL stays on the invalid group route. That way the test proves the invalid group ID triggers the intended error state, not just any generic error on the page.
Suggested implementation:
// Invalid group ID handling
When("User navigates to group details with invalid ID", async ({ page }) => {
await page.goto("/sbom-groups/invalid-group-id-12345");
});
Then("An error state is displayed for the invalid group", async ({ page }) => {
// Assert we stay on the invalid group route
await expect(page).toHaveURL(/\/sbom-groups\/invalid-group-id-12345$/);
// Assert the dedicated SBOM group error UI is shown
const sbomGroupError = page.getByTestId("sbom-group-error");
await expect(sbomGroupError).toBeVisible();
});
Then("The SBOM Groups table shows all groups", async ({ page }) => {
const table = page.getByRole("treegrid", { name: "sbom-groups-table" });
await expect(table).toBeVisible();
const rows = table.getByRole("row");
await expect(rows.first()).toBeVisible();
});
Then(- Ensure the SBOM group error UI in the application uses
data-testid="sbom-group-error"on the container element (e.g., the alert or error panel). If a different test id or fixed heading text is already used (such as"sbom-group-not-found-heading"or a specific error message string), update the selector in this step definition to match the existing UI. - If the invalid-group route differs (for example, it includes a workspace/org prefix or uses a different path segment), adjust the regular expression in
toHaveURLto match the actual route shape.
| @@ -447,7 +445,6 @@ When( | |||
| Then( | |||
Contributor
There was a problem hiding this comment.
suggestion (testing): SBOM count assertion doesn’t distinguish zero vs non-zero counts or missing content
The Then "The SBOM count is displayed for group {string}" step only asserts that text matching "\d+ SBOMs?" is visible, which passes for 0 SBOMs and could miss cases where the count is absent but similar text appears. If the behavior requires at least one SBOM, please assert on the parsed numeric value (e.g., > 0). Otherwise, add a dedicated scenario for 0 SBOMs so both zero and non-zero states are explicitly covered.
Suggested implementation:
Then(
"The SBOM count is displayed for group {string}",
async ({ page }, groupName: string) => {
const groupRow = page.getByRole("row", { name: new RegExp(groupName, "i") });
await expect(groupRow).toBeVisible();
const sbomCountLocator = groupRow.getByText(/\d+\s+SBOMs?/);
const sbomCountText = await sbomCountLocator.textContent();
expect(sbomCountText).not.toBeNull();
const match = sbomCountText!.match(/(\d+)\s+SBOMs?/);
expect(match).not.toBeNull();
const sbomCount = Number.parseInt(match![1], 10);
expect(sbomCount).toBeGreaterThan(0);
},
);- Ensure the DOM structure for the SBOM count matches the locator assumptions (count text within the group row and formatted like
"N SBOMs"). If the count is in a different element (e.g., a specific column), adjustsbomCountLocatoraccordingly (e.g.,groupRow.getByRole("cell", { name: /\d+\s+SBOMs?/ })). - If the product must support a valid
0 SBOMsstate, add a dedicated scenario to your feature file (e.g.,"The SBOM count shows zero SBOMs for group {string}") that assertssbomCount === 0instead of> 0, using the same parsing logic.
mrrajan
force-pushed
the
TC-3811
branch
2 times, most recently
from
0cf670a to
a8cbc46
Compare
Add missing E2E test coverage for SBOM Groups feature: - Product label badge visibility in list and detail pages - Hierarchical tree expand/collapse behavior - Parent group selection in create and edit flows - Invalid group ID error handling - SBOM count display in group list - Breadcrumb navigation on detail page - Edit group parent assignment and removal - Sorting by name column Implements TC-3811 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Assisted-by: Claude Code
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: mrrajan <86094767+mrrajan@users.noreply.github.com.>
Signed-off-by: mrrajan <86094767+mrrajan@users.noreply.github.com.>
Signed-off-by: mrrajan <86094767+mrrajan@users.noreply.github.com.>
Signed-off-by: mrrajan <86094767+mrrajan@users.noreply.github.com.>
Signed-off-by: mrrajan <86094767+mrrajan@users.noreply.github.com.>
Signed-off-by: mrrajan <86094767+mrrajan@users.noreply.github.com.>
Signed-off-by: mrrajan <86094767+mrrajan@users.noreply.github.com.>
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.
Summary
Scenarios Added
Not Implemented (features not yet in codebase)
Test plan
npm run e2e:test:uito verify all scenarios pass.claude/shared/bdd-standards.md)Implements TC-3811
Summary by Sourcery
Tests: