Open
Conversation
Deploying irenestaging with
|
| Latest commit: |
6d961e3
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f19ba7c3.irenestaging.pages.dev |
| Branch Preview URL: | https://feature-sbom-testcases.irenestaging.pages.dev |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR adds comprehensive Cypress testing infrastructure for SBOM features by introducing 20+ Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/components/sbom/app-list/action/index.hbs (1)
16-27:⚠️ Potential issue | 🟡 MinorStatic
aria-labelon all menu items breaks screen-reader differentiation.
aria-label='sbom app action menu item'is a static string applied to every iteration of the{{#eachthis.menuItems}}loop. When@button={{it.button}}causes the item to render as a button,aria-labeloverrides the accessible name derived from<li.text@PrimaryText={{it.label}} />, making every option announce identically to screen readers. This is a meaningful accessibility regression.If the intent is purely a Cypress selector, replace it with a
data-test-cyattribute (consistent with the rest of this PR) and drop thearia-labelentirely, or use the item's own label:🛡️ Proposed fix
- aria-label='sbom app action menu item' + data-test-cy='sbomApp-actionMenuItem'Or, if an
aria-labelis genuinely desired for accessibility:- aria-label='sbom app action menu item' + aria-label={{it.label}}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/sbom/app-list/action/index.hbs` around lines 16 - 27, The static aria-label on each akm.listItem inside the {{`#each` this.menuItems}} loop overrides the accessible name and makes all options announce identically; update the template so you remove the hard-coded aria-label and either (A) add a testing attribute like data-test-cy="sbomApp-actionMenuItem" instead (consistent with existing PR test attributes) or (B) if an aria-label is truly needed, make it dynamic using the menu item label (e.g., set aria-label to the item's label via it.label) and only apply it when rendering buttons (check it.button) so the accessible name from li.text `@primaryText`={{it.label}} is not incorrectly overridden.
🧹 Nitpick comments (7)
app/components/sbom/scan-details/file-scan-summary/index.hbs (1)
1-11: Inconsistentdata-test-cynaming conventions across attributes.
sbom-metadata-containeruses kebab-case whilesbomscandetails-filesummarygroupuses no separators. Consider standardizing on one convention (e.g., kebab-case throughout) for easier Cypress selector maintenance.Also, Line 11 has a trailing blank line before the closing
>— minor whitespace nit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/sbom/scan-details/file-scan-summary/index.hbs` around lines 1 - 11, The data-test-cy attributes in the AkStack elements are inconsistent—change all selectors to a single kebab-case convention (e.g., keep data-test-cy="sbom-metadata-container" and rename data-test-cy="sbomscandetails-filesummarygroup" to a kebab-case form like "sbom-scan-details-file-summary-group") and update any corresponding data-test-* attributes (e.g., data-test-sbomScanDetails-fileSummaryGroup -> data-test-sbom-scan-details-file-summary-group) to match; also remove the stray whitespace before the inner AkStack opening tag's closing ">" so there is no trailing blank space in that element declaration.cypress/tests/sbom.spec.ts (3)
18-20:NETWORK_WAIT_OPTSis already exported fromSbomPageActions.ts— import it instead of re-declaring.This duplicates the constant from
cypress/support/Actions/common/SbomPageActions.ts(lines 9–11).-const NETWORK_WAIT_OPTS = { - timeout: 60000, -}; +import { NETWORK_WAIT_OPTS } from '../support/Actions/common/SbomPageActions';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cypress/tests/sbom.spec.ts` around lines 18 - 20, The test file re-declares NETWORK_WAIT_OPTS instead of using the exported constant; remove the local declaration of NETWORK_WAIT_OPTS and import the existing exported constant from the support module (the export in SbomPageActions.ts) at the top of cypress/tests/sbom.spec.ts, then replace usages to reference the imported NETWORK_WAIT_OPTS so there is a single source-of-truth.
25-25: Blanketuncaught:exceptionsuppression hides real application errors.
Cypress.on('uncaught:exception', () => false)silently swallows every unhandled error from the application, including legitimate bugs. If this is needed to work around a known issue, consider filtering by error message or limiting the scope to the specific test that needs it.Example: scoped suppression
Cypress.on('uncaught:exception', (err) => { // Ignore known benign error if (err.message.includes('specific known error message')) { return false; } // Let other errors fail the test return true; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cypress/tests/sbom.spec.ts` at line 25, Replace the global blanket suppression for uncaught exceptions (Cypress.on('uncaught:exception')) with a scoped or filtered handler: inside the specific test or beforeEach that needs the workaround, install a Cypress.on('uncaught:exception', (err) => ...) that checks err.message (or other err properties) for the known benign error text and returns false only for that match, otherwise return true so real errors surface; alternatively remove the global handler entirely and add a scoped cy.on/Cypress.on in the single test that requires suppression to limit impact.
157-272: Consider splitting the SBOM Report Flow test into smaller, focused tests.This single
itblock (lines 165–272) covers uploading an app, intercepting WebSocket events, reloading, finding the SBOM project, navigating to past analyses, generating a report, validating generation, and downloading two report formats. If any step fails, the failure message won't clearly indicate which phase broke.Breaking it into smaller tests (e.g., separate upload, report generation, and download tests) with shared state via
beforehooks would improve debuggability and failure isolation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cypress/tests/sbom.spec.ts` around lines 157 - 272, The long "SBOM Report Flow" spec (describe 'SBOM Report Flow' / it 'opens Past SBOM Analyses and validates report generation flow') should be split into focused tests: one test for upload and websocket handling that uses uploadAppActions.initiateViaSystemUpload and the cy.interceptWsMessage handler to capture and alias uploadedFile/uploadedAppPrjID/uploadedAppSBPrjId in a before() or beforeEach(), a second test to navigate to Past SBOM Analyses and validate the SBOM list (using cy.get('@uploadedAppSBPrjId') and the existing selectors like sbomApp-row-..., sbomScan-row), and a third test for report generation and downloads that invokes sbomPageActions.generatePDFReport(), validateReportGenerated(), downloadSBOMAppReport('pdf'|'cyclonedx_json_file') and asserts network intercepts (`@sbPdfDownload`, `@sbCycloneDxJsonDownload`). Move shared intercepts (cy.intercept, cy.interceptWsMessage) into the before hook and rely on Cypress aliases to pass state between tests to improve isolation and clearer failure points.cypress/support/Actions/common/SbomPageActions.ts (3)
9-11:NETWORK_WAIT_OPTSis duplicated insbom.spec.ts.This constant (line 9–11) is also defined identically in
cypress/tests/sbom.spec.ts(line 18–20). Since it's already exported from this file, the spec file should import it instead of re-declaring.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cypress/support/Actions/common/SbomPageActions.ts` around lines 9 - 11, Remove the duplicate constant declaration by deleting the redundant NETWORK_WAIT_OPTS in the spec and import the exported NETWORK_WAIT_OPTS from SbomPageActions instead; update the spec file (sbom.spec.ts) to import { NETWORK_WAIT_OPTS } from the module that exports it and use that imported constant wherever NETWORK_WAIT_OPTS is referenced to avoid duplicated definitions.
246-308:validateComponentTypeFilteredRowsandvalidateDependencyTypeFilteredRowsare near-identical.These two methods (lines 246–258 and 296–308) share the exact same logic — only the method name differs. Consider extracting a shared helper to reduce duplication:
Proposed refactor
+ private validateFilteredRowsContain(type: string) { + cy.get('body').then((body) => { + const rowCount = body.find('[data-test-sbomComponent-row]').length; + + if (rowCount > 0) { + cy.get('[data-test-sbomComponent-row]') + .should('have.length.greaterThan', 0) + .each((row) => { + cy.wrap(row).invoke('text').should('contain', type); + }); + } + }); + } + validateComponentTypeFilteredRows(type: string) { - cy.get('body').then((body) => { - const rowCount = body.find('[data-test-sbomComponent-row]').length; - - if (rowCount > 0) { - cy.get('[data-test-sbomComponent-row]') - .should('have.length.greaterThan', 0) - .each((row) => { - cy.wrap(row).invoke('text').should('contain', type); - }); - } - }); + this.validateFilteredRowsContain(type); } + validateDependencyTypeFilteredRows(type: string) { - cy.get('body').then((body) => { - const rowCount = body.find('[data-test-sbomComponent-row]').length; - - if (rowCount > 0) { - cy.get('[data-test-sbomComponent-row]') - .should('have.length.greaterThan', 0) - .each((row) => { - cy.wrap(row).invoke('text').should('contain', type); - }); - } - }); + this.validateFilteredRowsContain(type); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cypress/support/Actions/common/SbomPageActions.ts` around lines 246 - 308, Both validateComponentTypeFilteredRows and validateDependencyTypeFilteredRows duplicate the same DOM-checking logic; extract a single reusable helper (e.g., validateFilteredRows(type: string)) that encapsulates the body lookup, rowCount check, and per-row text assertion against the provided type (using the existing selector '[data-test-sbomComponent-row]'). Replace the two original methods so they simply call validateFilteredRows(type) to keep behavior identical (including the conditional rowCount > 0 guard and .should('have.length.greaterThan', 0) assertion).
466-476: Hardcoded strings instead ofcyTranslatefor component summary labels.
'Component Type','Dependency Type','Status', and'SECURE'are hardcoded rather than usingcyTranslate(). If these labels are localized in the application, the tests will break when running against a different locale.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cypress/support/Actions/common/SbomPageActions.ts` around lines 466 - 476, In validateComponentDetailsPage(), replace the hardcoded visible text values with calls to cyTranslate so tests work with localized labels: change componentSummary('Component Type'), componentSummary('Dependency Type'), componentSummary('Status') and componentStatus('SECURE') to use cyTranslate(...) results (e.g. cyTranslate('Component Type').then(t => this.componentSummary(t).should('be.visible')) or chain appropriately) so the translated strings are passed into componentSummary and componentStatus instead of literal English text.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/components/ak-breadcrumbs/auto-trail/index.hbs`:
- Line 21: Remove the invalid dynamic attribute name "item-key-{{item.route}}"
from the ak-breadcrumbs auto-trail template and rely on the existing static
attribute data-test-ak-breadcrumbs-auto-trail-item-key='{{item.route}}' instead;
specifically, edit the auto-trail template where item elements are rendered
(look for the list item that references item.route) and delete the literal
attribute item-key-{{item.route}} so each breadcrumb only has the proper static
attribute with the dynamic value.
In `@app/components/sbom/scan-details/component-tree/index.hbs`:
- Around line 110-114: The data-test-cy attribute is accidentally inside a
Handlebars comment and therefore not rendered; update the AkIcon invocation
(AkIcon) so that data-test-cy="component-tree-node-expand-icon" is a real
attribute on the AkIcon element (not inside {{! ... }}), ensuring the attribute
is emitted to the DOM for Cypress to select.
In `@cypress/support/Actions/common/SbomPageActions.ts`:
- Around line 204-232: In validateMetadataDynamic, sbomFile.status is a numeric
enum and must be converted to the UI display string before asserting; update the
code that builds metadataFields to map sbomFile.status through the same mapping
used in UploadAppActions (SB_FILE_STATUS) so the status assertion compares the
human-readable label rather than the raw number, then leave the other fields
(sbomFile.completed_at and sbomFile.id) unchanged and continue asserting via
this.metadataField(...).should('contain', ...).
- Around line 349-362: In generatePDFReport replace the unsafe use of
cy.findAllByTestId('sbomReportList-reportGenerateBtn').click() by selecting a
single element first — call .first() on the result before .click() so only the
intended generate button is clicked; update the chain in the generatePDFReport
method to use .first() prior to .click() while keeping the subsequent assertions
(findByText checks) unchanged.
- Around line 339-343: downloadSBOMAppReport uses cy.findAllByTestId which can
return multiple elements; calling .click() on that collection will fail if more
than one matches. Update the downloadSBOMAppReport function to target a single
element (e.g., call .first() on the result of
cy.findAllByTestId(`sbomReportList-reportDownloadBtn-${reportType}`) before
.click()) so the click is performed on one element and avoid Cypress
multiple-element click errors.
- Around line 235-245: The failing pattern is passing DEFAULT_ASSERT_OPTS as a
second argument to .should(), which is treated as an expected value and not
timeout; update the selector helper(s) (e.g., componentTypePopover,
componentTypeFilterIcon, componentTypeRadio and any other cy.get-returning
helpers used at the noted sites) to accept an options param
(Partial<Cypress.Timeoutable>) and forward it into the underlying cy.get call,
then replace calls like this.componentTypePopover().should('be.visible',
DEFAULT_ASSERT_OPTS) with this.componentTypePopover({ timeout:
DEFAULT_ASSERT_OPTS.timeout }).should('be.visible'); apply the same change for
the usages referenced (lines ~238, 243, 277, 286, 467) so timeouts are passed to
cy.get instead of .should().
In `@cypress/support/Actions/common/UploadAppActions.ts`:
- Around line 113-115: The intercept URL is built incorrectly:
API_ROUTES.sbomFile.route contains a glob (`/api/v2/sb_files/*`) so
concatenating `/${sbomPrjFileID}` yields a mismatched pattern; update the
intercept to use the actual path by removing the wildcard or using a dedicated
route that includes the id (change how `sbFileURL` is constructed so it results
in `/api/v2/sb_files/{sbomPrjFileID}`), then call
`cy.intercept(sbFileURL).as('uploadedAppSBFileReq')` so the
`cy.wait('@uploadedAppSBFileReq')` will match. Reference symbols:
`API_ROUTES.sbomFile.route`, `sbomPrjFileID`, `sbFileURL`, `cy.intercept`,
`uploadedAppSBFileReq`.
In `@cypress/tests/sbom.spec.ts`:
- Around line 242-244: Replace the hard-coded cy.wait(...) calls (the
cy.wait(3000) and cy.wait(2000) calls adjacent to cy.reload()) with
deterministic waits: either stub and wait on a network intercept (use
cy.intercept(...) and cy.wait('@alias') for the scan request/response) or
poll/assert the DOM for a completion indicator (e.g., cy.get(...) or
cy.contains(...) that reflects scan status) before calling cy.reload() or
proceeding; update the test to remove cy.wait(...) and instead wait on the
intercept alias or a specific DOM assertion that indicates the scan is finished.
- Around line 355-371: The inner describe.skip('SBOM Page - Pagination Tests')
contains a beforeEach that duplicates the outer describe's setup (the outer
beforeEach registers intercepts, calls loginActions.loginWithCredAndSaveSession,
and visits APPLICATION_ROUTES.sbom); remove the inner beforeEach block so tests
inherit the outer setup (or alternatively move the pagination describe to
top-level if it needs independent setup). Specifically, delete the nested
beforeEach that re-registers API_ROUTES intercepts and re-calls
loginActions.loginWithCredAndSaveSession/cy.visit(APPLICATION_ROUTES.sbom) so
only the outer beforeEach runs.
- Around line 373-385: The test is asserting the table element exists via
table().should('have.length.greaterThan', 0) but that only checks the table
node, not its data rows; update the assertions to check the row collection
inside the table (e.g., select table body rows via table().find('tbody tr') or a
row helper) and assert its length is greater than 0 after clicking next/prev;
apply the same change for the other occurrences noted (the assertions near lines
referencing table() at the later checks) and keep using firstRow(), nextBtn,
prevBtn and paginationLabel for the rest of the flow.
- Line 383: The assertion is comparing a string to a Cypress Chainable
(cy.get('@initialRow') / cy.get('@page1Url')) which always passes; replace the
direct .should('not.equal', cy.get('@initialRow')) usage by unwrapping the alias
with .then(...) and compare the invoked text to the unwrapped string.
Specifically, for firstRow() use firstRow().invoke('text').then((text) => {
compare text against the value yielded by cy.get('@initialRow') }) and do the
same pattern for the check that uses cy.get('@page1Url'), ensuring you compare
primitive string values (e.g., with expect(...).not.equal(...) or an equivalent
assertion) rather than a Chainable.
In `@mirage/factories/sbom-scan-summary.ts`:
- Around line 4-13: The factory fields in SBOM_SCAN_SUMMARY_FACTORY_DEF are
invoking faker at module load (e.g., component_count: faker.number.int(1000)),
causing the same values for every instance; change each field to a function
returning the faker value (e.g., component_count: () => faker.number.int(1000))
so values are evaluated per-instance, and keep exporting the Factory via
Factory.extend(SBOM_SCAN_SUMMARY_FACTORY_DEF) as before.
---
Outside diff comments:
In `@app/components/sbom/app-list/action/index.hbs`:
- Around line 16-27: The static aria-label on each akm.listItem inside the
{{`#each` this.menuItems}} loop overrides the accessible name and makes all
options announce identically; update the template so you remove the hard-coded
aria-label and either (A) add a testing attribute like
data-test-cy="sbomApp-actionMenuItem" instead (consistent with existing PR test
attributes) or (B) if an aria-label is truly needed, make it dynamic using the
menu item label (e.g., set aria-label to the item's label via it.label) and only
apply it when rendering buttons (check it.button) so the accessible name from
li.text `@primaryText`={{it.label}} is not incorrectly overridden.
---
Nitpick comments:
In `@app/components/sbom/scan-details/file-scan-summary/index.hbs`:
- Around line 1-11: The data-test-cy attributes in the AkStack elements are
inconsistent—change all selectors to a single kebab-case convention (e.g., keep
data-test-cy="sbom-metadata-container" and rename
data-test-cy="sbomscandetails-filesummarygroup" to a kebab-case form like
"sbom-scan-details-file-summary-group") and update any corresponding data-test-*
attributes (e.g., data-test-sbomScanDetails-fileSummaryGroup ->
data-test-sbom-scan-details-file-summary-group) to match; also remove the stray
whitespace before the inner AkStack opening tag's closing ">" so there is no
trailing blank space in that element declaration.
In `@cypress/support/Actions/common/SbomPageActions.ts`:
- Around line 9-11: Remove the duplicate constant declaration by deleting the
redundant NETWORK_WAIT_OPTS in the spec and import the exported
NETWORK_WAIT_OPTS from SbomPageActions instead; update the spec file
(sbom.spec.ts) to import { NETWORK_WAIT_OPTS } from the module that exports it
and use that imported constant wherever NETWORK_WAIT_OPTS is referenced to avoid
duplicated definitions.
- Around line 246-308: Both validateComponentTypeFilteredRows and
validateDependencyTypeFilteredRows duplicate the same DOM-checking logic;
extract a single reusable helper (e.g., validateFilteredRows(type: string)) that
encapsulates the body lookup, rowCount check, and per-row text assertion against
the provided type (using the existing selector '[data-test-sbomComponent-row]').
Replace the two original methods so they simply call validateFilteredRows(type)
to keep behavior identical (including the conditional rowCount > 0 guard and
.should('have.length.greaterThan', 0) assertion).
- Around line 466-476: In validateComponentDetailsPage(), replace the hardcoded
visible text values with calls to cyTranslate so tests work with localized
labels: change componentSummary('Component Type'), componentSummary('Dependency
Type'), componentSummary('Status') and componentStatus('SECURE') to use
cyTranslate(...) results (e.g. cyTranslate('Component Type').then(t =>
this.componentSummary(t).should('be.visible')) or chain appropriately) so the
translated strings are passed into componentSummary and componentStatus instead
of literal English text.
In `@cypress/tests/sbom.spec.ts`:
- Around line 18-20: The test file re-declares NETWORK_WAIT_OPTS instead of
using the exported constant; remove the local declaration of NETWORK_WAIT_OPTS
and import the existing exported constant from the support module (the export in
SbomPageActions.ts) at the top of cypress/tests/sbom.spec.ts, then replace
usages to reference the imported NETWORK_WAIT_OPTS so there is a single
source-of-truth.
- Line 25: Replace the global blanket suppression for uncaught exceptions
(Cypress.on('uncaught:exception')) with a scoped or filtered handler: inside the
specific test or beforeEach that needs the workaround, install a
Cypress.on('uncaught:exception', (err) => ...) that checks err.message (or other
err properties) for the known benign error text and returns false only for that
match, otherwise return true so real errors surface; alternatively remove the
global handler entirely and add a scoped cy.on/Cypress.on in the single test
that requires suppression to limit impact.
- Around line 157-272: The long "SBOM Report Flow" spec (describe 'SBOM Report
Flow' / it 'opens Past SBOM Analyses and validates report generation flow')
should be split into focused tests: one test for upload and websocket handling
that uses uploadAppActions.initiateViaSystemUpload and the cy.interceptWsMessage
handler to capture and alias uploadedFile/uploadedAppPrjID/uploadedAppSBPrjId in
a before() or beforeEach(), a second test to navigate to Past SBOM Analyses and
validate the SBOM list (using cy.get('@uploadedAppSBPrjId') and the existing
selectors like sbomApp-row-..., sbomScan-row), and a third test for report
generation and downloads that invokes sbomPageActions.generatePDFReport(),
validateReportGenerated(), downloadSBOMAppReport('pdf'|'cyclonedx_json_file')
and asserts network intercepts (`@sbPdfDownload`, `@sbCycloneDxJsonDownload`). Move
shared intercepts (cy.intercept, cy.interceptWsMessage) into the before hook and
rely on Cypress aliases to pass state between tests to improve isolation and
clearer failure points.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
app/components/ak-breadcrumbs/auto-trail/index.hbsapp/components/project-list/app-platform/index.hbsapp/components/project-list/header/index.hbsapp/components/sbom/app-list/action/index.hbsapp/components/sbom/app-list/header/index.hbsapp/components/sbom/app-list/index.hbsapp/components/sbom/app-platform/index.hbsapp/components/sbom/app-scan/list/index.hbsapp/components/sbom/app-scan/list/view-report/index.hbsapp/components/sbom/empty-loading-view/index.hbsapp/components/sbom/scan-details/component-list/dependency-type-header/index.hbsapp/components/sbom/scan-details/component-list/index.hbsapp/components/sbom/scan-details/component-list/type-header/index.hbsapp/components/sbom/scan-details/component-tree/index.hbsapp/components/sbom/scan-details/file-scan-summary/index.hbsapp/components/sbom/scan-details/index.hbsapp/components/sbom/scan-details/overview/index.hbsapp/components/sbom/scan-report-drawer/report-list/item/index.hbscypress/support/Actions/common/SbomPageActions.tscypress/support/Actions/common/UploadAppActions.tscypress/support/Mirage/factories.config.tscypress/support/Websocket/index.tscypress/support/api.routes.tscypress/support/application.routes.tscypress/tests/auth.spec.tscypress/tests/sbom.spec.tsmirage/factories/sbom-scan-summary.ts
Comment on lines
+339
to
+343
| downloadSBOMAppReport(reportType: 'pdf' | 'cyclonedx_json_file') { | ||
| cy.findAllByTestId(`sbomReportList-reportDownloadBtn-${reportType}`) | ||
| .should('be.visible') | ||
| .click({ force: true }); | ||
| } |
There was a problem hiding this comment.
findAllByTestId + .click() without .first() — Cypress will error if multiple elements match.
cy.findAllByTestId(...) can return multiple elements. Calling .click() on a multi-element result requires { multiple: true } or you should use .first() to target a single element. If the page has more than one download button for the same report type (e.g., from multiple past analyses), this will throw a Cypress error.
Proposed fix
downloadSBOMAppReport(reportType: 'pdf' | 'cyclonedx_json_file') {
cy.findAllByTestId(`sbomReportList-reportDownloadBtn-${reportType}`)
+ .first()
.should('be.visible')
.click({ force: true });
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| downloadSBOMAppReport(reportType: 'pdf' | 'cyclonedx_json_file') { | |
| cy.findAllByTestId(`sbomReportList-reportDownloadBtn-${reportType}`) | |
| .should('be.visible') | |
| .click({ force: true }); | |
| } | |
| downloadSBOMAppReport(reportType: 'pdf' | 'cyclonedx_json_file') { | |
| cy.findAllByTestId(`sbomReportList-reportDownloadBtn-${reportType}`) | |
| .first() | |
| .should('be.visible') | |
| .click({ force: true }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cypress/support/Actions/common/SbomPageActions.ts` around lines 339 - 343,
downloadSBOMAppReport uses cy.findAllByTestId which can return multiple
elements; calling .click() on that collection will fail if more than one
matches. Update the downloadSBOMAppReport function to target a single element
(e.g., call .first() on the result of
cy.findAllByTestId(`sbomReportList-reportDownloadBtn-${reportType}`) before
.click()) so the click is performed on one element and avoid Cypress
multiple-element click errors.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
cypress/tests/sbom.spec.ts (1)
26-26: Swallowing all uncaught exceptions may hide real application bugs.Returning
falsefor alluncaught:exceptionevents prevents Cypress from failing on any application error. This can mask legitimate bugs. Consider filtering to only ignore known, expected exceptions.💡 Example: Filter specific exceptions
Cypress.on('uncaught:exception', (err) => { // Ignore specific known errors if (err.message.includes('ResizeObserver loop')) { return false; } // Let other errors fail the test return true; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cypress/tests/sbom.spec.ts` at line 26, Replace the unconditional Cypress.on('uncaught:exception', () => false) handler with a filtered handler that inspects the error (the err parameter provided to the 'uncaught:exception' event) and only returns false for specific known/expected error messages (e.g., check err.message for strings like 'ResizeObserver loop' or any other project-specific transient errors), otherwise return true so Cypress fails the test on unexpected exceptions; update the handler in sbom.spec.ts where Cypress.on('uncaught:exception' is currently registered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cypress/tests/sbom.spec.ts`:
- Line 371: The test suite contains an invalid token: the `+` prefix before the
test block declaration `+describe('Navigation - Next/Previous', () => {` is a
syntax error; remove the leading `+` so the block reads `describe('Navigation -
Next/Previous', () => {` and ensure the corresponding callback for `describe`
(the anonymous arrow function) is properly closed with matching `})`/`});` and
that any nested `it`/`context` blocks inside the `describe` remain syntactically
balanced (verify the `describe` block name `Navigation - Next/Previous` is used
as shown).
- Line 10: Remove the unused import "time" from 'echarts' in sbom.spec.ts;
locate the import statement "import { time } from 'echarts';" and delete it (or
remove only the "time" specifier if other imports are added) so the test file no
longer contains an unused import.
---
Nitpick comments:
In `@cypress/tests/sbom.spec.ts`:
- Line 26: Replace the unconditional Cypress.on('uncaught:exception', () =>
false) handler with a filtered handler that inspects the error (the err
parameter provided to the 'uncaught:exception' event) and only returns false for
specific known/expected error messages (e.g., check err.message for strings like
'ResizeObserver loop' or any other project-specific transient errors), otherwise
return true so Cypress fails the test on unexpected exceptions; update the
handler in sbom.spec.ts where Cypress.on('uncaught:exception' is currently
registered.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
cypress/support/Actions/common/SbomPageActions.ts (4)
406-418:⚠️ Potential issue | 🟡 MinorMap SBOM status to the UI label before asserting metadata.
Line 407 uses raw
sbomFile.status; UI metadata usually shows a status label, not numeric enum values.Suggested fix
- const metadataFields = { - [cyTranslate('status')]: sbomFile.status, + const metadataFields = { + [cyTranslate('status')]: SB_FILE_STATUS[sbomFile.status], [cyTranslate('sbomModule.generatedDate')]: sbomFile.completed_at, [cyTranslate('file')]: sbomFile.id, };#!/bin/bash # Verify available status mapping and current metadata assertion usage. rg -n "SB_FILE_STATUS|sbomFile\\.status|metadataFields" \ cypress/support/Actions/common/UploadAppActions.ts \ cypress/support/Actions/common/SbomPageActions.ts🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cypress/support/Actions/common/SbomPageActions.ts` around lines 406 - 418, The metadata assertion is using raw sbomFile.status (in metadataFields / the loop) but the UI shows the mapped status label; update the code that builds metadataFields to convert sbomFile.status into its display label (reuse the existing status mapping used elsewhere, e.g., SB_FILE_STATUS or a helper like formatSbomStatus) before assigning [cyTranslate('status')], so that this.metadataField(label).should('contain', expectedValue) compares against the UI string rather than the numeric enum; ensure you reference the same mapping function/constant used by UploadAppActions or other tests to keep labels consistent.
231-231:⚠️ Potential issue | 🟠 Major
.should(..., options)does not apply timeout; options are ignored.At these lines, timeout/options are passed to
.should()instead of the underlying query command.Suggested fix
- this.componentOverviewTab().should('be.visible', { timeout: 10000 }); + this.componentOverviewTab({ timeout: 10000 }).should('be.visible'); - this.componentTypePopover().should('be.visible', DEFAULT_ASSERT_OPTS); + this.componentTypePopover({ timeout: DEFAULT_ASSERT_OPTS.timeout }).should('be.visible'); - this.componentTypeRadio(type) - .should('exist', DEFAULT_ASSERT_OPTS) + this.componentTypeRadio(type, { timeout: DEFAULT_ASSERT_OPTS.timeout }) + .should('exist') .click({ force: true }); - this.dependencyTypePopover().should('be.visible', DEFAULT_ASSERT_OPTS); + this.dependencyTypePopover({ timeout: DEFAULT_ASSERT_OPTS.timeout }).should('be.visible'); - this.dependencyTypeRadio(type) - .should('exist', DEFAULT_ASSERT_OPTS) + this.dependencyTypeRadio(type, { timeout: DEFAULT_ASSERT_OPTS.timeout }) + .should('exist') .click({ force: true });// Supporting helper signature updates (same file) componentOverviewTab(opts?: Partial<Cypress.Timeoutable>) { return cy.get('[data-test-sbomComponentDetails-tab="overview"]', opts); } componentTypePopover(opts?: Partial<Cypress.Timeoutable>) { return cy.get('[data-test-sbom-scanDetails-componentTypeHeader-popover]', opts); } componentTypeRadio(type: string, opts?: Partial<Cypress.Timeoutable>) { return cy.get(`[data-test-sbom-scanDetails-componentTypeHeader-radio="${type}"]`, opts); } dependencyTypePopover(opts?: Partial<Cypress.Timeoutable>) { return cy.get('[data-test-sbom-scanDetails-dependencyTypeHeader-popover]', opts); } dependencyTypeRadio(type: string, opts?: Partial<Cypress.Timeoutable>) { return cy.findByTestId(`dependency-type-filter-radio-${type}`, opts); }#!/bin/bash # Verify misuse of options as second argument to .should(...) rg -nP "\\.should\\(\\s*'[^']+'\\s*,\\s*(DEFAULT_ASSERT_OPTS|\\{\\s*timeout\\s*:)" cypress/support/Actions/common/SbomPageActions.tsAlso applies to: 431-431, 439-440, 478-478, 486-487
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cypress/support/Actions/common/SbomPageActions.ts` at line 231, The test is passing timeout/options as the second argument to .should(...), which Cypress ignores; update calls to pass timeout into the underlying query helper instead (e.g. call componentOverviewTab({ timeout: 10000 }).should('be.visible') rather than componentOverviewTab().should('be.visible', { timeout: 10000 })), and do the same for other occurrences noted (lines referencing componentTypePopover, componentTypeRadio, dependencyTypePopover, dependencyTypeRadio). Ensure each helper invocation forwards the options into its cy.get/findByTestId call so the timeout is applied before .should() is asserted.
139-142:⚠️ Potential issue | 🟡 MinorTarget a single element before clicking
findAllByTestId(...)results.These calls can fail when multiple buttons match.
Suggested fix
downloadSBOMAppReport(reportType: 'pdf' | 'cyclonedx_json_file') { cy.findAllByTestId(`sbomReportList-reportDownloadBtn-${reportType}`) + .first() .should('be.visible') .click({ force: true }); } generatePDFReport() { cy.findAllByTestId('sbomReportList-reportGenerateBtn') + .first() .should('not.be.disabled') .click({ force: true });#!/bin/bash # Verify multi-element click patterns in this helper. rg -nP "findAllByTestId\\([^\\)]*\\)\\s*\\n\\s*\\.should\\([^\\)]*\\)\\s*\\n\\s*\\.click\\(" cypress/support/Actions/common/SbomPageActions.tsAlso applies to: 149-152
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cypress/support/Actions/common/SbomPageActions.ts` around lines 139 - 142, The test uses cy.findAllByTestId(`sbomReportList-reportDownloadBtn-${reportType}`) and then clicks, which can fail when multiple matches exist; change to target a single element (either use cy.findByTestId(...) if a single element is expected, or call .first() / .eq(index) on the result before asserting visibility and clicking) — e.g., replace the chain with cy.findAllByTestId(`sbomReportList-reportDownloadBtn-${reportType}`).first().should('be.visible').click({ force: true }); and apply the same change for the other occurrence around the block that matches lines 149-152.
184-190:⚠️ Potential issue | 🟠 MajorFix tab selector key casing to match the actual data-test attribute.
These selectors use
sbomcomponentdetails(lowercasec) while Line 327 usessbomComponentDetails. This inconsistency can breakvalidateComponentDetailsPage().Suggested fix
- componentOverviewTab() { - return cy.get('[data-test-sbomcomponentdetails-tab="overview"]'); + componentOverviewTab() { + return cy.get('[data-test-sbomComponentDetails-tab="overview"]'); } - componentVulnTab() { - return cy.get('[data-test-sbomcomponentdetails-tab="vulnerabilities"]'); + componentVulnTab() { + return cy.get('[data-test-sbomComponentDetails-tab="vulnerabilities"]'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cypress/support/Actions/common/SbomPageActions.ts` around lines 184 - 190, The selectors in componentOverviewTab() and componentVulnTab() use the wrong casing for the data-test attribute ("sbomcomponentdetails") causing validateComponentDetailsPage() to fail; update the selectors in those functions to use the correct attribute key ("sbomComponentDetails") so the cy.get calls in componentOverviewTab and componentVulnTab match the actual data-test used elsewhere (e.g., where validateComponentDetailsPage references sbomComponentDetails).cypress/tests/sbom.spec.ts (1)
242-243:⚠️ Potential issue | 🟠 MajorReplace fixed sleeps with deterministic waits.
Lines 242/253/263 still use time-based waits. This remains flaky/slow and was already flagged in prior review.
Suggested fix
- // Wait for SBOM Scan to complete - cy.wait(3000); // open the first (latest) report in the list cy.findAllByTestId('sbomScan-viewReportBtn') .first() .should('be.visible') .click({ force: true }); sbomPageActions.generatePDFReport(); sbomPageActions.validateReportGenerated(); - cy.wait(3000); // wait for report generation to complete - // Download PDF report sbomPageActions.downloadSBOMAppReport('pdf'); cy.wait('@sbPdfDownload', { timeout: 30000 }) .its('response.statusCode') .should('equal', 200); - // Wait before second click - cy.wait(2000); - // Download JSON sbomPageActions.downloadSBOMAppReport('cyclonedx_json_file');#!/bin/bash # Verify no hard-coded waits remain in this spec. rg -nP "cy\\.wait\\((2000|3000)\\)" cypress/tests/sbom.spec.tsAlso applies to: 253-254, 263-264
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cypress/tests/sbom.spec.ts` around lines 242 - 243, Tests are using fixed sleeps (cy.wait(3000)) to wait for the "SBOM Scan" which makes them flaky; replace each cy.wait(3000) (and other hard-coded waits at the same spots) with deterministic synchronization — either intercept the network call that indicates SBOM scan completion and wait on an alias (cy.intercept(...).as('sbomScan'); cy.wait('@sbomScan')) or poll for a UI/state change (e.g., cy.get('<scan-complete-selector>').should('be.visible') or cy.contains('Scan complete').should('exist')) in the sbom.spec.ts test where the comment "Wait for SBOM Scan to complete" appears, replacing the cy.wait calls at those locations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cypress/tests/sbom.spec.ts`:
- Line 26: The current global handler Cypress.on('uncaught:exception', () =>
false) suppresses all runtime errors; change the handler in sbom.spec.ts to
inspect the exception (use the handler signature (err, runnable) or (error)
depending on test runtime), match error.message or error.stack against known
benign patterns (e.g., /ResizeObserver loop limit exceeded/,
/SomeKnownLibraryError/, etc.), and only return false for those matched patterns
while returning true/letting the error propagate for any other errors so real
failures still fail the test; update any test-specific setup to list the allowed
regexes and use Cypress.on('uncaught:exception', (err) => { if
(allowedPatterns.some(p => p.test(err.message || err.stack || ''))) return
false; }) as the implementation reference.
---
Duplicate comments:
In `@cypress/support/Actions/common/SbomPageActions.ts`:
- Around line 406-418: The metadata assertion is using raw sbomFile.status (in
metadataFields / the loop) but the UI shows the mapped status label; update the
code that builds metadataFields to convert sbomFile.status into its display
label (reuse the existing status mapping used elsewhere, e.g., SB_FILE_STATUS or
a helper like formatSbomStatus) before assigning [cyTranslate('status')], so
that this.metadataField(label).should('contain', expectedValue) compares against
the UI string rather than the numeric enum; ensure you reference the same
mapping function/constant used by UploadAppActions or other tests to keep labels
consistent.
- Line 231: The test is passing timeout/options as the second argument to
.should(...), which Cypress ignores; update calls to pass timeout into the
underlying query helper instead (e.g. call componentOverviewTab({ timeout: 10000
}).should('be.visible') rather than componentOverviewTab().should('be.visible',
{ timeout: 10000 })), and do the same for other occurrences noted (lines
referencing componentTypePopover, componentTypeRadio, dependencyTypePopover,
dependencyTypeRadio). Ensure each helper invocation forwards the options into
its cy.get/findByTestId call so the timeout is applied before .should() is
asserted.
- Around line 139-142: The test uses
cy.findAllByTestId(`sbomReportList-reportDownloadBtn-${reportType}`) and then
clicks, which can fail when multiple matches exist; change to target a single
element (either use cy.findByTestId(...) if a single element is expected, or
call .first() / .eq(index) on the result before asserting visibility and
clicking) — e.g., replace the chain with
cy.findAllByTestId(`sbomReportList-reportDownloadBtn-${reportType}`).first().should('be.visible').click({
force: true }); and apply the same change for the other occurrence around the
block that matches lines 149-152.
- Around line 184-190: The selectors in componentOverviewTab() and
componentVulnTab() use the wrong casing for the data-test attribute
("sbomcomponentdetails") causing validateComponentDetailsPage() to fail; update
the selectors in those functions to use the correct attribute key
("sbomComponentDetails") so the cy.get calls in componentOverviewTab and
componentVulnTab match the actual data-test used elsewhere (e.g., where
validateComponentDetailsPage references sbomComponentDetails).
In `@cypress/tests/sbom.spec.ts`:
- Around line 242-243: Tests are using fixed sleeps (cy.wait(3000)) to wait for
the "SBOM Scan" which makes them flaky; replace each cy.wait(3000) (and other
hard-coded waits at the same spots) with deterministic synchronization — either
intercept the network call that indicates SBOM scan completion and wait on an
alias (cy.intercept(...).as('sbomScan'); cy.wait('@sbomScan')) or poll for a
UI/state change (e.g., cy.get('<scan-complete-selector>').should('be.visible')
or cy.contains('Scan complete').should('exist')) in the sbom.spec.ts test where
the comment "Wait for SBOM Scan to complete" appears, replacing the cy.wait
calls at those locations.
Yibaebi
force-pushed
the
feature/sbom-testcases
branch
from
41449b3 to
56515e2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
cypress/tests/sbom.spec.ts (1)
10-10:⚠️ Potential issue | 🟡 MinorRemove unused import
timefrom 'echarts'.This import is not used anywhere in the test file and should be removed.
Proposed fix
-import { time } from 'echarts';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cypress/tests/sbom.spec.ts` at line 10, The test file imports an unused symbol time from 'echarts' which should be removed to clean up unused imports; open the cypress tests file and delete the import statement "import { time } from 'echarts';" (or remove time from the import list if other symbols are added later) so only necessary imports remain in sbom.spec.ts.
🧹 Nitpick comments (1)
cypress/support/Actions/common/SbomPageActions.ts (1)
448-460: Duplicate implementations:validateComponentTypeFilteredRowsandvalidateDependencyTypeFilteredRows.These two methods have identical logic (confirmed by SonarCloud). Consider extracting a shared helper to reduce duplication.
Proposed refactor
+ /** + * `@name` validateFilteredRows + * `@description` Validates filtered rows contain the specified type value + */ + private validateFilteredRows(type: string) { + cy.get('body').then((body) => { + const rowCount = body.find('[data-test-sbomComponent-row]').length; + + if (rowCount > 0) { + cy.get('[data-test-sbomComponent-row]') + .should('have.length.greaterThan', 0) + .each((row) => { + cy.wrap(row).invoke('text').should('contain', type); + }); + } + }); + } validateComponentTypeFilteredRows(type: string) { - cy.get('body').then((body) => { - const rowCount = body.find('[data-test-sbomComponent-row]').length; - - if (rowCount > 0) { - cy.get('[data-test-sbomComponent-row]') - .should('have.length.greaterThan', 0) - .each((row) => { - cy.wrap(row).invoke('text').should('contain', type); - }); - } - }); + this.validateFilteredRows(type); } validateDependencyTypeFilteredRows(type: string) { - cy.get('body').then((body) => { - const rowCount = body.find('[data-test-sbomComponent-row]').length; - - if (rowCount > 0) { - cy.get('[data-test-sbomComponent-row]') - .should('have.length.greaterThan', 0) - .each((row) => { - cy.wrap(row).invoke('text').should('contain', type); - }); - } - }); + this.validateFilteredRows(type); }Also applies to: 497-509
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cypress/support/Actions/common/SbomPageActions.ts` around lines 448 - 460, Both validateComponentTypeFilteredRows and validateDependencyTypeFilteredRows contain identical logic; extract that logic into a reusable helper (e.g., validateFilteredRowsByType or similar) and have both original methods delegate to it. The helper should accept the row selector and the expected type, perform the body.find('[data-test-...-row]').length check, and if > 0 iterate over cy.get(selector).each(...) asserting the row text contains the type. Replace calls in validateComponentTypeFilteredRows (using '[data-test-sbomComponent-row]') and validateDependencyTypeFilteredRows (using '[data-test-sbomDependency-row]') to call the new helper to remove duplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cypress/tests/sbom.spec.ts`:
- Line 243: Replace the hard-coded cy.wait(...) calls in sbom.spec.ts with
deterministic waits: intercept the report-generation/status network request
using cy.intercept(...) and wait on the alias (cy.wait('@reportStatus')) or poll
the DOM for the readiness indicator (e.g.,
cy.get('<ready-indicator-selector>').should('be.visible') or .should('contain',
'Complete')) instead of cy.wait(3000)/cy.wait(2000); update the three
occurrences where cy.wait(...) is used to either wait on the intercepted network
alias or assert the specific DOM state that signals the report is ready.
---
Duplicate comments:
In `@cypress/tests/sbom.spec.ts`:
- Line 10: The test file imports an unused symbol time from 'echarts' which
should be removed to clean up unused imports; open the cypress tests file and
delete the import statement "import { time } from 'echarts';" (or remove time
from the import list if other symbols are added later) so only necessary imports
remain in sbom.spec.ts.
---
Nitpick comments:
In `@cypress/support/Actions/common/SbomPageActions.ts`:
- Around line 448-460: Both validateComponentTypeFilteredRows and
validateDependencyTypeFilteredRows contain identical logic; extract that logic
into a reusable helper (e.g., validateFilteredRowsByType or similar) and have
both original methods delegate to it. The helper should accept the row selector
and the expected type, perform the body.find('[data-test-...-row]').length
check, and if > 0 iterate over cy.get(selector).each(...) asserting the row text
contains the type. Replace calls in validateComponentTypeFilteredRows (using
'[data-test-sbomComponent-row]') and validateDependencyTypeFilteredRows (using
'[data-test-sbomDependency-row]') to call the new helper to remove duplication.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b40b7585-4788-419d-a934-bb75a25378d0
📒 Files selected for processing (26)
app/components/ak-breadcrumbs/auto-trail/index.hbsapp/components/project-list/app-platform/index.hbsapp/components/sbom/app-list/action/index.hbsapp/components/sbom/app-list/header/index.hbsapp/components/sbom/app-list/index.hbsapp/components/sbom/app-platform/index.hbsapp/components/sbom/app-scan/list/index.hbsapp/components/sbom/app-scan/list/view-report/index.hbsapp/components/sbom/empty-loading-view/index.hbsapp/components/sbom/scan-details/component-list/dependency-type-header/index.hbsapp/components/sbom/scan-details/component-list/index.hbsapp/components/sbom/scan-details/component-list/type-header/index.hbsapp/components/sbom/scan-details/component-tree/index.hbsapp/components/sbom/scan-details/file-scan-summary/index.hbsapp/components/sbom/scan-details/index.hbsapp/components/sbom/scan-details/overview/index.hbsapp/components/sbom/scan-report-drawer/report-list/item/index.hbscypress/support/Actions/common/SbomPageActions.tscypress/support/Actions/common/UploadAppActions.tscypress/support/Mirage/factories.config.tscypress/support/Websocket/index.tscypress/support/api.routes.tscypress/support/application.routes.tscypress/tests/auth.spec.tscypress/tests/sbom.spec.tsmirage/factories/sbom-scan-summary.ts
✅ Files skipped from review due to trivial changes (18)
- cypress/support/application.routes.ts
- app/components/project-list/app-platform/index.hbs
- app/components/sbom/app-scan/list/view-report/index.hbs
- app/components/sbom/app-platform/index.hbs
- app/components/sbom/scan-details/index.hbs
- app/components/sbom/app-scan/list/index.hbs
- app/components/sbom/app-list/action/index.hbs
- app/components/sbom/scan-details/overview/index.hbs
- app/components/sbom/scan-details/component-list/index.hbs
- app/components/sbom/app-list/header/index.hbs
- cypress/tests/auth.spec.ts
- app/components/sbom/app-list/index.hbs
- app/components/sbom/empty-loading-view/index.hbs
- app/components/sbom/scan-details/component-list/dependency-type-header/index.hbs
- app/components/sbom/scan-details/component-list/type-header/index.hbs
- app/components/sbom/scan-report-drawer/report-list/item/index.hbs
- app/components/sbom/scan-details/file-scan-summary/index.hbs
- app/components/sbom/scan-details/component-tree/index.hbs
🚧 Files skipped from review as they are similar to previous changes (4)
- app/components/ak-breadcrumbs/auto-trail/index.hbs
- cypress/support/Actions/common/UploadAppActions.ts
- mirage/factories/sbom-scan-summary.ts
- cypress/support/Websocket/index.ts
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
cypress/support/Actions/common/SbomPageActions.ts (2)
109-123:⚠️ Potential issue | 🟡 MinorClick a single report action, not a collection.
Both helpers call
.click()onfindAllByTestId(...). Once the page has more than one matching report row/button, Cypress will throw because the subject contains multiple elements. Narrow the query before clicking.🛠️ Proposed fix
downloadSBOMAppReport(reportType: 'pdf' | 'cyclonedx_json_file') { cy.findAllByTestId(`sbomReportList-reportDownloadBtn-${reportType}`) + .first() .should('be.visible') .click({ force: true }); } @@ generatePDFReport() { cy.findAllByTestId('sbomReportList-reportGenerateBtn') + .first() .should('not.be.disabled') .click({ force: true });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cypress/support/Actions/common/SbomPageActions.ts` around lines 109 - 123, The current helpers use cy.findAllByTestId(...) which returns multiple elements and causes Cypress to fail when more than one match exists; in downloadSBOMAppReport and generatePDFReport narrow the selector before clicking (e.g., use cy.findByTestId(...) to target a single element, or chain .first()/.eq(n) or scope the query inside a specific row via .within()) so the click operates on a single button (refer to sbomReportList-reportDownloadBtn-{reportType} and sbomReportList-reportGenerateBtn identifiers and the functions downloadSBOMAppReport/generatePDFReport).
235-243:⚠️ Potential issue | 🟠 MajorThese timeout objects are never applied.
should('be.visible', DEFAULT_ASSERT_OPTS)andshould('exist', DEFAULT_ASSERT_OPTS)do not set a Cypress timeout; the second argument is treated as assertion data. These helpers still use the default command timeout, so the extra retry budget is lost.Example fix
- cy.findByTestId('sbom-component-type-filter-popover').should( - 'be.visible', - DEFAULT_ASSERT_OPTS - ); + cy.findByTestId( + 'sbom-component-type-filter-popover', + DEFAULT_ASSERT_OPTS + ).should('be.visible'); - cy.findByTestId(`sbom-component-type-filter-radio-${type}`) - .should('exist', DEFAULT_ASSERT_OPTS) + cy.findByTestId( + `sbom-component-type-filter-radio-${type}`, + DEFAULT_ASSERT_OPTS + ) + .should('exist') .click({ force: true });Also applies to: 249-253, 290-307
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cypress/support/Actions/common/SbomPageActions.ts` around lines 235 - 243, The assertions pass DEFAULT_ASSERT_OPTS to should(...) which is ignored; move the timeout into the DOM query so Cypress actually uses the extra retry budget. For each occurrence (e.g., in openComponentTypeFilter use cy.findByTestId('sbom-component-type-filter-popover', DEFAULT_ASSERT_OPTS).should('be.visible') instead of should('be.visible', DEFAULT_ASSERT_OPTS); apply the same change for the other instances mentioned (lines ~249-253 and ~290-307) replacing should(..., DEFAULT_ASSERT_OPTS) with passing DEFAULT_ASSERT_OPTS as the second argument to cy.findByTestId / other query calls and keep the should(...) without the timeout arg.cypress/tests/sbom.spec.ts (2)
25-25:⚠️ Potential issue | 🟠 MajorScope
uncaught:exceptionsuppression to known benign errors.Line 25 currently returns
falsefor every runtime exception in this spec, which can hide real SBOM UI regressions and produce false-green runs. Keep the ignore list explicit and let unknown exceptions fail the test.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cypress/tests/sbom.spec.ts` at line 25, The current global handler Cypress.on('uncaught:exception', () => false) suppresses all runtime errors; replace it with a scoped filter that only returns false for known benign SBOM UI error patterns and re-throws or lets unknown errors fail the test. Add a descriptive constant (e.g., KNOWN_SBOM_ERRORS or knownSbomErrorPatterns) and in the 'uncaught:exception' callback check err.message (or err.stack) against those patterns, returning false only on matches and otherwise allowing the exception to propagate so real regressions fail the spec.
238-260:⚠️ Potential issue | 🟠 MajorReplace these fixed sleeps with the signals this block already has.
Lines 239, 250, and 260 are brittle and slow. This flow already has deterministic checkpoints (
sbomScan-viewReportBtn,validateReportGenerated(),@sbPdfDownload,@sbCycloneDxJsonDownload), so waiting on those instead will make the suite much less flaky.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cypress/tests/sbom.spec.ts` around lines 238 - 260, Remove the fixed cy.wait(3000/2000) sleeps and rely on the existing deterministic checks: wait for the sbomScan-viewReportBtn visibility before clicking (cy.findAllByTestId('sbomScan-viewReportBtn').first().should('be.visible')), rely on sbomPageActions.validateReportGenerated() to block until generation completes, and wait on the network aliases (cy.wait('@sbPdfDownload') and/or cy.wait('@sbCycloneDxJsonDownload')) instead of the intermediate sleeps; in short, delete the three cy.wait(...) calls and ensure the flow uses the visibility assertion, validateReportGenerated(), and the '@sbPdfDownload'/'@sbCycloneDxJsonDownload' cy.waits as your synchronization points.
🧹 Nitpick comments (1)
cypress/support/Actions/common/SbomPageActions.ts (1)
260-272: Extract the duplicated filtered-row assertion helper.
validateComponentTypeFilteredRows()andvalidateDependencyTypeFilteredRows()are the same implementation with different type inputs. A shared helper will keep future assertion fixes from drifting between the two filter paths.Also applies to: 317-329
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cypress/support/Actions/common/SbomPageActions.ts` around lines 260 - 272, Two methods, validateComponentTypeFilteredRows and validateDependencyTypeFilteredRows, duplicate the same assertion logic; extract a shared helper (e.g., validateFilteredRows(type: string, testId: string) or validateTypeFilteredRows(type: string)) that contains the cy.get body -> rowCount check and the cy.findAllByTestId(...).each(...) assertion, then replace both validateComponentTypeFilteredRows and validateDependencyTypeFilteredRows to delegate to that helper (use the existing test ids like 'sbom-component-row' and the dependency row test id when calling it) so future fixes are made in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/components/ak-pagination/index.hbs`:
- Line 18: The hard-coded generic aria-labels in the ak-pagination template are
overriding the real, localized accessible names (for per-page selector,
page-range display, and prev/next buttons) and are being used as test hooks;
remove those aria-label attributes and instead let the elements expose their
rendered text/translation as the accessible name (e.g., remove aria-label on the
per-page select, the page-range span, and the previous/next buttons), reuse the
existing per-page translation helper for the select label, and add dedicated
test selectors (data-cy or data-test attributes) to the relevant elements so
Cypress can target them without changing accessibility semantics; ensure no
element replaces visible content with a generic aria-label and that prev/next
still use their localized labels.
In `@cypress/support/Actions/common/SbomPageActions.ts`:
- Around line 26-28: The selector in componentDetailsTab builds
sbom-component-details-tab-"overview" because the template string includes extra
quotes; update the cy.findByTestId call in componentDetailsTab to remove the
embedded double quotes around ${tabId} so it constructs
sbom-component-details-tab-overview (i.e., use
sbom-component-details-tab-${tabId} in the template) to match the emitted
{{item.id}} and allow the detail-page tab assertions to find the element.
In `@cypress/tests/sbom.spec.ts`:
- Around line 368-371: The timeout option is being passed to should() which
treats it as an assertion value and is ignored; update the call so the timeout
is passed to the query step instead — modify
sbomPageActions.componentDetailsTab(tabId) to accept and forward options (e.g.,
componentDetailsTab(tabId, { timeout: 10000 })) and then call
.should('be.visible'), or if componentDetailsTab does not accept options, apply
the timeout on the underlying query (use cy.findByTestId(...) with { timeout:
10000 }) before chaining .should('be.visible').
---
Duplicate comments:
In `@cypress/support/Actions/common/SbomPageActions.ts`:
- Around line 109-123: The current helpers use cy.findAllByTestId(...) which
returns multiple elements and causes Cypress to fail when more than one match
exists; in downloadSBOMAppReport and generatePDFReport narrow the selector
before clicking (e.g., use cy.findByTestId(...) to target a single element, or
chain .first()/.eq(n) or scope the query inside a specific row via .within()) so
the click operates on a single button (refer to
sbomReportList-reportDownloadBtn-{reportType} and
sbomReportList-reportGenerateBtn identifiers and the functions
downloadSBOMAppReport/generatePDFReport).
- Around line 235-243: The assertions pass DEFAULT_ASSERT_OPTS to should(...)
which is ignored; move the timeout into the DOM query so Cypress actually uses
the extra retry budget. For each occurrence (e.g., in openComponentTypeFilter
use cy.findByTestId('sbom-component-type-filter-popover',
DEFAULT_ASSERT_OPTS).should('be.visible') instead of should('be.visible',
DEFAULT_ASSERT_OPTS); apply the same change for the other instances mentioned
(lines ~249-253 and ~290-307) replacing should(..., DEFAULT_ASSERT_OPTS) with
passing DEFAULT_ASSERT_OPTS as the second argument to cy.findByTestId / other
query calls and keep the should(...) without the timeout arg.
In `@cypress/tests/sbom.spec.ts`:
- Line 25: The current global handler Cypress.on('uncaught:exception', () =>
false) suppresses all runtime errors; replace it with a scoped filter that only
returns false for known benign SBOM UI error patterns and re-throws or lets
unknown errors fail the test. Add a descriptive constant (e.g.,
KNOWN_SBOM_ERRORS or knownSbomErrorPatterns) and in the 'uncaught:exception'
callback check err.message (or err.stack) against those patterns, returning
false only on matches and otherwise allowing the exception to propagate so real
regressions fail the spec.
- Around line 238-260: Remove the fixed cy.wait(3000/2000) sleeps and rely on
the existing deterministic checks: wait for the sbomScan-viewReportBtn
visibility before clicking
(cy.findAllByTestId('sbomScan-viewReportBtn').first().should('be.visible')),
rely on sbomPageActions.validateReportGenerated() to block until generation
completes, and wait on the network aliases (cy.wait('@sbPdfDownload') and/or
cy.wait('@sbCycloneDxJsonDownload')) instead of the intermediate sleeps; in
short, delete the three cy.wait(...) calls and ensure the flow uses the
visibility assertion, validateReportGenerated(), and the
'@sbPdfDownload'/'@sbCycloneDxJsonDownload' cy.waits as your synchronization
points.
---
Nitpick comments:
In `@cypress/support/Actions/common/SbomPageActions.ts`:
- Around line 260-272: Two methods, validateComponentTypeFilteredRows and
validateDependencyTypeFilteredRows, duplicate the same assertion logic; extract
a shared helper (e.g., validateFilteredRows(type: string, testId: string) or
validateTypeFilteredRows(type: string)) that contains the cy.get body ->
rowCount check and the cy.findAllByTestId(...).each(...) assertion, then replace
both validateComponentTypeFilteredRows and validateDependencyTypeFilteredRows to
delegate to that helper (use the existing test ids like 'sbom-component-row' and
the dependency row test id when calling it) so future fixes are made in one
place.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 710dcc5c-3d73-4b53-ac2f-c36467b1dbbf
📒 Files selected for processing (16)
app/components/ak-breadcrumbs/auto-trail/index.hbsapp/components/ak-pagination/index.hbsapp/components/sbom/app-list/index.hbsapp/components/sbom/component-details/index.hbsapp/components/sbom/component-details/summary/index.hbsapp/components/sbom/component-status/index.hbsapp/components/sbom/scan-details/component-list/dependency-type-header/index.hbsapp/components/sbom/scan-details/component-list/index.hbsapp/components/sbom/scan-details/component-list/type-header/index.hbsapp/components/sbom/scan-details/component-tree/index.hbsapp/components/sbom/scan-details/file-scan-summary/index.hbsapp/components/sbom/scan-details/index.hbsapp/components/sbom/summary-header/index.hbscypress/support/Actions/common/SbomPageActions.tscypress/tests/dynamic-scan.spec.tscypress/tests/sbom.spec.ts
✅ Files skipped from review due to trivial changes (11)
- app/components/sbom/component-details/summary/index.hbs
- app/components/sbom/component-status/index.hbs
- app/components/sbom/component-details/index.hbs
- app/components/sbom/scan-details/index.hbs
- app/components/sbom/summary-header/index.hbs
- app/components/sbom/scan-details/file-scan-summary/index.hbs
- app/components/sbom/app-list/index.hbs
- app/components/sbom/scan-details/component-list/dependency-type-header/index.hbs
- app/components/sbom/scan-details/component-list/index.hbs
- app/components/sbom/scan-details/component-list/type-header/index.hbs
- app/components/sbom/scan-details/component-tree/index.hbs
🚧 Files skipped from review as they are similar to previous changes (1)
- app/components/ak-breadcrumbs/auto-trail/index.hbs
Yibaebi
force-pushed
the
feature/sbom-testcases
branch
from
a8ce1f3 to
eea2207
Compare
Yibaebi
force-pushed
the
feature/sbom-testcases
branch
from
eea2207 to
c9acc13
Compare
Irene
|
||||||||||||||||||||||||||||||||||||||||
| Project |
Irene
|
| Branch Review |
feature/sbom-testcases
|
| Run status |
|
| Run duration | 03m 27s |
| Commit |
|
| Committer | Elliot Yibaebi |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
2
|
Flaky
|
0
|
Pending
|
1
|
Skipped
|
1
|
Passing
|
47
|
| View all changes introduced in this branch ↗︎ | |
Tests for review
dynamic-scan.spec.ts • 1 failed test
| Test | Artifacts | |
|---|---|---|
| Dynamic Scan > it tests dynamic scan for an apk file: 132571 |
Test Replay
Screenshots
|
|
sbom.spec.ts • 1 failed test
| Test | Artifacts | |
|---|---|---|
| SBOM Page > SBOM Report Detail Page > validates overview counts from API response |
Test Replay
Screenshots
|
|
Yibaebi
force-pushed
the
feature/sbom-testcases
branch
from
c9acc13 to
6d961e3
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
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.
SBOM UI Testcases improvements