Skip to content
This repository was archived by the owner on Sep 11, 2025. It is now read-only.

Tests for SBOM Vulnerabilities Summary Panel#40

Merged
mrrajan merged 10 commits intotrustification:mainfrom
mrrajan:sbom_vulns
Apr 11, 2025
Merged

Tests for SBOM Vulnerabilities Summary Panel#40
mrrajan merged 10 commits intotrustification:mainfrom
mrrajan:sbom_vulns

Conversation

@mrrajan
Copy link
Copy Markdown
Collaborator

@mrrajan mrrajan commented Mar 24, 2025

Added test for SBOM Explorer Vulnerabilities tab to verify the vulnerabilities count

  • Validation for individual severity count
  • Comparison for PieChart total value to individual severity counts

test-results.zip

@mrrajan mrrajan added the WIP label Mar 24, 2025
@mrrajan mrrajan removed the WIP label Mar 25, 2025
@mrrajan mrrajan marked this pull request as ready for review March 25, 2025 11:00
@vobratil
Copy link
Copy Markdown
Contributor

I've run this a couple of times against the non-AWS instance we're using right now and the tests act a little bit flaky. They fail inconsistently, typically with:

Call log:
      - waiting for getByLabel('Contents') to be detached


       at ../tests/ui/helpers/DetailsPage.ts:47

      45 |   async waitForData() {
      46 |     const spinner = this.page.getByLabel("Contents");
    > 47 |     await spinner.waitFor({ state: "detached" });
         |                   ^
      48 |   }
      49 |
      50 |   //Verifies the Page loads with data
        at DetailsPage.waitForData (/home/vobratil/Development/trustify-tests/tests/ui/helpers/DetailsPage.ts:47:19)
        at SearchPage.dedicatedSearch (/home/vobratil/Development/trustify-tests/tests/ui/helpers/SearchPage.ts:20:23)

Perhaps we should somehow increase the wait time, as it's likely to be longer on a "real" instance than a local one.

@mrrajan
Copy link
Copy Markdown
Collaborator Author

mrrajan commented Mar 26, 2025

@vobratil The failure is unfortunate - I have tested this on a regular OCP environment before raising this PR and attached the traces with the description. you could check them with the comment npx playwright show-trace <path/to/traces.zip>.

However, The error appears to have been caused by the async call. I have addressed it now, so it should be working. . Please check and let me know.

Comment thread tests/ui/features/@sbom-explorer/sbom-explorer.step.ts
Comment thread tests/ui/features/@sbom-explorer/sbom-explorer.step.ts
Comment thread tests/ui/features/@sbom-explorer/sbom-explorer.step.ts
Copy link
Copy Markdown
Contributor

@vobratil vobratil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I re-ran the tests against one of our currently running instances and one of the tests was failing for me. I'm not sure if it has something to do with the bug mentioned on triage yesterday, but I've added some logging suggestions which could / should be applied generally across the PR.

Comment thread tests/ui/helpers/DetailsPage.ts Outdated
Comment thread tests/ui/features/@sbom-explorer/sbom-explorer.feature
Comment thread tests/ui/helpers/DetailsPage.ts Outdated
Comment thread tests/ui/helpers/DetailsPage.ts
Rajan Ravi added 8 commits April 8, 2025 12:52
Signed-off-by: Rajan Ravi <rravi@rravi-thinkpadp1gen4i.bengluru.csb>
Signed-off-by: Rajan Ravi <rravi@rravi-thinkpadp1gen4i.bengluru.csb>
Signed-off-by: Rajan Ravi <rravi@rravi-thinkpadp1gen4i.bengluru.csb>
Signed-off-by: Rajan Ravi <rravi@rravi-thinkpadp1gen4i.bengluru.csb>
Signed-off-by: Rajan Ravi <rravi@rravi-thinkpadp1gen4i.bengluru.csb>
Signed-off-by: Rajan Ravi <rravi@rravi-thinkpadp1gen4i.bengluru.csb>
Signed-off-by: Rajan Ravi <rravi@rravi-thinkpadp1gen4i.bengluru.csb>
Signed-off-by: Rajan Ravi <rravi@rravi-thinkpadp1gen4i.bengluru.csb>
Signed-off-by: Rajan Ravi <rravi@rravi-thinkpadp1gen4i.bengluru.csb>
Signed-off-by: Rajan Ravi <rravi@rravi-thinkpadp1gen4i.bengluru.csb>
@mrrajan mrrajan requested a review from carlosthe19916 April 11, 2025 11:41
Copy link
Copy Markdown
Contributor

@vobratil vobratil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've given it one more look and it looks good to me at this stage. In my opinion ready to merge.

@mrrajan mrrajan merged commit 503a2f7 into trustification:main Apr 11, 2025
4 checks passed
Copy link
Copy Markdown
Member

@carlosthe19916 carlosthe19916 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mrrajan I arrived late to the review. Yet I wanted to share my thoughts on this PR.
We can address my comments in future PRs. We are going to evolve iterating the code anyway :)


Scenario Outline: View SBOM Overview
Given User visits SBOM details Page of "<sbomName>"
Given An ingested "<sbomType>" SBOM "<sbomName>" is available
Copy link
Copy Markdown
Member

@carlosthe19916 carlosthe19916 Apr 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The <sbomType> is not used neither declared in the Examples section of the Scenario definition. I wonder if we should remove it.


Scenario Outline: View SBOM Info (Metadata)
Given User visits SBOM details Page of "<sbomName>"
Given An ingested "<sbomType>" SBOM "<sbomName>" is available
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above, <sbomType> is not used nor needed.

"An ingested {string} SBOM {string} containing Vulnerabilities",
async ({ page }, _sbomType, sbomName) => {
const searchPage = new SearchPage(page);
await searchPage.dedicatedSearch("SBOMs", sbomName);
Copy link
Copy Markdown
Member

@carlosthe19916 carlosthe19916 Apr 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the dedicatedSearch method receives a "string" that represents the page we want to see. I think the first parameter does not belong there. What about:

// Here we define that when we instantiate a Search Object we want to move the page to the SBOMs page.
const searchPage = new SearchPage(page, "SBOMs");
await searchPage.search("sbom_name");

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functionality wise, this wont make much difference. But, it makes sense for code maintenance and readability. I will try including this change as part of next PR.

const searchPage = new SearchPage(page);
await searchPage.dedicatedSearch("SBOMs", sbomName);
const element = await page.locator(
`xpath=(//tr[contains(.,'${sbomName}')]/td[@data-label='Vulnerabilities']/div)[1]`
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see many xpath selectors which by default is ok. But:

  • We can decide to keep working with xpath OR
  • We can also change certain parts of the UI's source code to make selection easier. E.g. in the past I faced a similar problem and I ended up adding aria selectors like in this PR https://github.com/trustification/trustify-ui/pull/313/files . Then use those aria selectors to avoid complex xpath selectors. As a bonus, the aria selectors are going to enrich the assistive technologies, such as screen readers in the UI.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In most of the conditions - I agree, we should consider aria-label or clean object selectors. But dynamic xpath's would be super helpful with reusable functions. For example, the pagination or per page selector validation dynamic xpath will be more useful. Let's consider where we can have dynamic xpath and static locators.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants