Skip to content

Conversation

@matejnesuta
Copy link
Contributor

@matejnesuta matejnesuta commented Sep 10, 2025

Summary by Sourcery

Introduce end-to-end BDD tests for the Search page and extend helper classes to support search, filtering, pagination, sorting, and tab interactions across Vulnerabilities, SBOMs, Packages, and Advisories.

New Features:

  • Add BDD feature file and step definitions for Search page scenarios.
  • Implement SearchPage helper with methods for navigation, search input, autocomplete, and result validation.
  • Add Tabs helper class for selecting and verifying tab states and badge counts.
  • Extend ToolbarTable helper with methods for column content checks, download link validation, pagination controls, date filtering, visibility assertions, and link verification.

Enhancements:

  • Improve existing ToolbarTable methods with explicit waits, adjustable timeouts, safe sorting logic, and a maxPages parameter for row retrieval.
  • Update DetailsPage header verification to wait for network idle and target explicit element selectors.

CI:

  • Enable screenshots on failure in Playwright configuration and adjust feature file discovery patterns.

@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Sep 10, 2025

Reviewer's Guide

This PR implements end-to-end BDD search tests by extending and refactoring UI helper classes for robust table and page interactions (pagination, filtering, sorting, autocomplete, tabs), adds new step definitions and feature files for search scenarios, and updates global test configuration to capture failure artifacts.

Sequence diagram for search autocomplete interaction

sequenceDiagram
  actor User
  participant "SearchPage"
  participant "Autocomplete Engine"
  User->>SearchPage: Type in search box
  SearchPage->>"Autocomplete Engine": Request suggestions
  "Autocomplete Engine"-->>SearchPage: Return suggestions
  SearchPage-->>User: Display suggestions
  User->>SearchPage: Select suggestion
  SearchPage->>SearchPage: Validate selection
  SearchPage-->>User: Show filtered results
Loading

Sequence diagram for table sorting and pagination in search results

sequenceDiagram
  actor User
  participant "ToolbarTable"
  User->>ToolbarTable: Click column to sort
  ToolbarTable->>ToolbarTable: Sort rows
  ToolbarTable-->>User: Display sorted table
  User->>ToolbarTable: Click next page
  ToolbarTable->>ToolbarTable: Load next page (maxPages limit)
  ToolbarTable-->>User: Display new page
Loading

Class diagram for updated UI helpers (ToolbarTable, SearchPage, Tabs, DetailsPage)

classDiagram
  class ToolbarTable {
    +verifyColumnContents()
    +verifyDownloadLinks()
    +verifyRowLimits()
    +verifyVisibility()
    +verifyDateFilters()
    +navigate()
    +verifyLinkPresence()
    +getTableRows(maxPages)
    +sortColumn()
    +waitForTableLoad()
    +waitForPaginationControls()
    +waitForSorting()
  }
  class SearchPage {
    +openSearchPage()
    +typeAutocomplete()
    +validateAutocompleteSuggestions()
    +countResultsByCategory()
  }
  class Tabs {
    +selectTab()
    +verifyTabVisibility()
    +verifySelectionState()
    +verifyBadgeCounts()
  }
  class DetailsPage {
    +verifyPageHeader()
  }
  ToolbarTable <|-- SearchPage
  SearchPage o-- Tabs
  SearchPage o-- ToolbarTable
  DetailsPage o-- ToolbarTable
Loading

File-Level Changes

Change Details Files
Extended ToolbarTable with new verification and navigation methods
  • Added verifyColumnContainsText and verifyColumnDoesNotContainText
  • Added verifyDownloadLink, verifyTableHasUpToRows, verifyTableIsVisible
  • Added filterByDate and clearFilter
  • Added openDetailsPage, goToNextPage, goToPreviousPage
  • Added verifyColumnContainsLink
e2e/tests/ui/helpers/ToolbarTable.ts
Refactored pagination and sorting logic in ToolbarTable
  • Introduced maxPages parameter and pageCount guard in getTableRows
  • Increased loading timeout for progress bar
  • Adjusted selectPerPage to wait for visibility before click
  • Updated sortColumn to use non-exact matching and button-specific clicks
  • Guarded against blank cells in sorting logic
e2e/tests/ui/helpers/ToolbarTable.ts
Added SearchPage helper methods for search page and autocomplete handling
  • Added open(), clickOnPageAction(), verifyPageHeader()
  • Added typeInSearchBox with networkidle wait
  • Added autoFillIsVisible, autoFillIsNotVisible, autoFillHasRelevantResults
  • Added totalAutoFillResults, autoFillCategoryCountsByHref, expectCategoriesWithinLimitByHref
e2e/tests/ui/helpers/SearchPage.ts
Introduced Tabs helper for tab interactions
  • Added selectTab, verifyTabIsSelected, verifyTabIsVisible, verifyTabIsNotVisible
  • Added verifyTabHasAtLeastResults to assert badge counts
e2e/tests/ui/helpers/Tabs.ts
Implemented BDD step definitions and feature files for comprehensive search scenarios
  • Updated details-page steps to use Tabs helper
  • Created search.step.ts with step implementations for search, filtering, sorting, pagination
  • Added search.feature outlining scenarios for tabs, download links, autocomplete, navigation
e2e/tests/ui/steps/details-page.ts
e2e/tests/ui/features/@search/search.step.ts
e2e/tests/ui/features/@search/search.feature
Enhanced DetailsPage header verification
  • Changed verifyPageHeader to wait for networkidle state
  • Switched to h1 locator with explicit timeout
e2e/tests/ui/helpers/DetailsPage.ts
Updated Playwright configuration for test artifacts and BDD setup
  • Enabled screenshot capture on failure
  • Adjusted BDD feature patterns
  • Retained trace on first retry and ignoreHTTPS setting
e2e/playwright.config.ts

Possibly linked issues

  • Migrate code from trustify #1: PR enhances UI helpers for pagination, filtering, and sorting, directly addressing the issue's aim to reduce duplicate code for these common e2e test logic areas.

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@matejnesuta matejnesuta marked this pull request as ready for review September 16, 2025 10:40
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes - here's some feedback:

  • The ToolbarTable helper methods use a lot of explicit any types—consider replacing those with stronger types (e.g. string) to improve type safety and clarity.
  • Selectors like "#autocomplete-search .pf-v6-c-menu" and table locators are repeated—extract them into shared constants or private helper methods to reduce duplication and ease maintenance.
  • Several step definitions bundle large conditional branches for different types; refactor these into parameterized or data-driven steps to make the BDD definitions more concise and maintainable.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The ToolbarTable helper methods use a lot of explicit any types—consider replacing those with stronger types (e.g. string) to improve type safety and clarity.
- Selectors like "#autocomplete-search .pf-v6-c-menu" and table locators are repeated—extract them into shared constants or private helper methods to reduce duplication and ease maintenance.
- Several step definitions bundle large conditional branches for different types; refactor these into parameterized or data-driven steps to make the BDD definitions more concise and maintainable.

## Individual Comments

### Comment 1
<location> `e2e/tests/ui/features/@search/search.step.ts:117` </location>
<code_context>
+  await detailsPage.verifyPageHeader(arg);
+});
+
+Then('the user should be able to filter {string}', async ({page}, arg: string) => {
+    const table = new ToolbarTable(page,getTableInfo(arg)[0]);
+  if (arg === "SBOMs"){
</code_context>

<issue_to_address>
**suggestion (testing):** Add negative filter tests to verify that filtering for non-existent values yields no results.

Consider adding tests that filter for values not present in the dataset to confirm the UI handles empty results as expected.

Suggested implementation:

```typescript
Then('the user should be able to filter {string}', async ({page}, arg: string) => {
    const table = new ToolbarTable(page,getTableInfo(arg)[0]);
  if (arg === "SBOMs"){
    await table.filterByDate("12/22/2025","12/22/2025");
    await table.verifyColumnDoesNotContainText("Name","quarkus-bom");
    await table.clearFilter();
    await table.verifyColumnContainsText("Name","quarkus-bom");

    // Negative filter test: filter for a non-existent SBOM name
    await table.filterByText("Name", "non-existent-sbom");
    await table.verifyColumnDoesNotContainText("Name", "non-existent-sbom");
    await table.verifyTableIsEmpty(); // Optionally verify table is empty
    await table.clearFilter();

  }else if (arg === "Vulnerabilities"){
    await page.getByLabel('Critical').check();
    await table.verifyColumnDoesNotContainText("ID","CVE-2022-45787");
    await table.clearFilter();
    await table.verifyColumnContainsText("ID","CVE-2022-45787");

    // Negative filter test: filter for a non-existent Vulnerability ID
    await table.filterByText("ID", "NON-EXISTENT-CVE");
    await table.verifyColumnDoesNotContainText("ID", "NON-EXISTENT-CVE");
    await table.verifyTableIsEmpty(); // Optionally verify table is empty
    await table.clearFilter();

  }else if (arg == "Packages"){

```

- Ensure that the `ToolbarTable` class has a `filterByText` and `verifyTableIsEmpty` method. If not, you will need to implement these helper methods.
- Add similar negative filter tests for "Packages" if applicable, using a column and value that do not exist.
</issue_to_address>

### Comment 2
<location> `e2e/tests/ui/features/@search/search.step.ts:161-166` </location>
<code_context>
+});
+
+
+Then('the {string} list should be sortable', async ({page}, arg: string) => {
+  var columns:string[] = getColumns(arg);
+  var id:string = getPaginationId(arg);
+
+  const table = new ToolbarTable(page,getTableInfo(arg)[0]);
+  await table.verifySorting(`xpath=//div[@id="${id}"]`,columns);
+});
+
</code_context>

<issue_to_address>
**suggestion (testing):** Add tests for sorting columns with mixed data types and blank cells.

Please add tests that verify sorting behavior for columns containing both valid values and blank cells, to confirm correct handling after the recent logic update.

Suggested implementation:

```typescript
Then('the {string} list should be sortable', async ({page}, arg: string) => {
  var columns:string[] = getColumns(arg);
  var id:string = getPaginationId(arg);

  const table = new ToolbarTable(page,getTableInfo(arg)[0]);
  await table.verifySorting(`xpath=//div[@id="${id}"]`,columns);
});

// Test sorting for columns with mixed data types and blank cells
Then('the {string} list should sort columns with mixed data types and blank cells correctly', async ({page}, arg: string) => {
  // Example: Assume getMixedTypeColumns returns columns with mixed types and blank cells for the given list
  var mixedColumns: string[] = getMixedTypeColumns(arg);
  var id: string = getPaginationId(arg);

  const table = new ToolbarTable(page, getTableInfo(arg)[0]);
  for (const column of mixedColumns) {
    // Optionally, you can pass a custom comparator or check for blank cell handling if verifySorting supports it
    await table.verifySorting(`xpath=//div[@id="${id}"]`, [column]);
  }
});

```

- You will need to implement or update the `getMixedTypeColumns(arg: string): string[]` function to return the relevant columns for each list that contain mixed data types and blank cells.
- Ensure your test data in the UI contains columns with both valid values and blank cells for the lists being tested.
- If `verifySorting` does not already check for blank cell handling, you may need to enhance it to do so.
</issue_to_address>

### Comment 3
<location> `e2e/tests/ui/features/@search/search.step.ts:209-212` </location>
<code_context>
+  await tabs.verifyTabHasAtLeastResults(arg,count);
+});
+
+Then('the autofill dropdown should display items matching the {string}', async ({page}, arg: string) => {
+  const searchPage = new SearchPage(page);
+  await searchPage.autoFillHasRelevantResults(arg);
+});
+
</code_context>

<issue_to_address>
**suggestion (testing):** Consider testing autocomplete with special characters and very long input strings.

Testing with these inputs will help catch potential UI issues and ensure the autocomplete remains reliable.

```suggestion
Then('the autofill dropdown should display items matching the {string}', async ({page}, arg: string) => {
  const searchPage = new SearchPage(page);
  await searchPage.autoFillHasRelevantResults(arg);
});

Then('the autofill dropdown should handle special characters input', async ({page}) => {
  const searchPage = new SearchPage(page);
  const specialCharsInput = '!@#$%^&*()_+-=[]{}|;:\'",.<>/?`~';
  await searchPage.autoFillHasRelevantResults(specialCharsInput);
});

Then('the autofill dropdown should handle very long input strings', async ({page}) => {
  const searchPage = new SearchPage(page);
  const longInput = 'a'.repeat(256); // 256 characters long
  await searchPage.autoFillHasRelevantResults(longInput);
});
```
</issue_to_address>

### Comment 4
<location> `e2e/tests/ui/features/@search/search.step.ts:162` </location>
<code_context>

</code_context>

<issue_to_address>
**issue (code-quality):** Use `const` or `let` instead of `var`. ([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/avoid-using-var))

<details><summary>Explanation</summary>`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code).
`let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than
function-scoped.

From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>

### Comment 5
<location> `e2e/tests/ui/features/@search/search.step.ts:163` </location>
<code_context>

</code_context>

<issue_to_address>
**issue (code-quality):** Use `const` or `let` instead of `var`. ([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/avoid-using-var))

<details><summary>Explanation</summary>`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code).
`let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than
function-scoped.

From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>

### Comment 6
<location> `e2e/tests/ui/features/@search/search.step.ts:179` </location>
<code_context>

</code_context>

<issue_to_address>
**issue (code-quality):** Use `const` or `let` instead of `var`. ([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/avoid-using-var))

<details><summary>Explanation</summary>`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code).
`let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than
function-scoped.

From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>

### Comment 7
<location> `e2e/tests/ui/features/@search/search.step.ts:188` </location>
<code_context>

</code_context>

<issue_to_address>
**issue (code-quality):** Use `const` or `let` instead of `var`. ([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/avoid-using-var))

<details><summary>Explanation</summary>`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code).
`let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than
function-scoped.

From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines 161 to 166
Then('the {string} list should be sortable', async ({page}, arg: string) => {
var columns:string[] = getColumns(arg);
var id:string = getPaginationId(arg);

const table = new ToolbarTable(page,getTableInfo(arg)[0]);
await table.verifySorting(`xpath=//div[@id="${id}"]`,columns);
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (testing): Add tests for sorting columns with mixed data types and blank cells.

Please add tests that verify sorting behavior for columns containing both valid values and blank cells, to confirm correct handling after the recent logic update.

Suggested implementation:

Then('the {string} list should be sortable', async ({page}, arg: string) => {
  var columns:string[] = getColumns(arg);
  var id:string = getPaginationId(arg);

  const table = new ToolbarTable(page,getTableInfo(arg)[0]);
  await table.verifySorting(`xpath=//div[@id="${id}"]`,columns);
});

// Test sorting for columns with mixed data types and blank cells
Then('the {string} list should sort columns with mixed data types and blank cells correctly', async ({page}, arg: string) => {
  // Example: Assume getMixedTypeColumns returns columns with mixed types and blank cells for the given list
  var mixedColumns: string[] = getMixedTypeColumns(arg);
  var id: string = getPaginationId(arg);

  const table = new ToolbarTable(page, getTableInfo(arg)[0]);
  for (const column of mixedColumns) {
    // Optionally, you can pass a custom comparator or check for blank cell handling if verifySorting supports it
    await table.verifySorting(`xpath=//div[@id="${id}"]`, [column]);
  }
});
  • You will need to implement or update the getMixedTypeColumns(arg: string): string[] function to return the relevant columns for each list that contain mixed data types and blank cells.
  • Ensure your test data in the UI contains columns with both valid values and blank cells for the lists being tested.
  • If verifySorting does not already check for blank cell handling, you may need to enhance it to do so.

Comment on lines 209 to 212
Then('the autofill dropdown should display items matching the {string}', async ({page}, arg: string) => {
const searchPage = new SearchPage(page);
await searchPage.autoFillHasRelevantResults(arg);
});
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (testing): Consider testing autocomplete with special characters and very long input strings.

Testing with these inputs will help catch potential UI issues and ensure the autocomplete remains reliable.

Suggested change
Then('the autofill dropdown should display items matching the {string}', async ({page}, arg: string) => {
const searchPage = new SearchPage(page);
await searchPage.autoFillHasRelevantResults(arg);
});
Then('the autofill dropdown should display items matching the {string}', async ({page}, arg: string) => {
const searchPage = new SearchPage(page);
await searchPage.autoFillHasRelevantResults(arg);
});
Then('the autofill dropdown should handle special characters input', async ({page}) => {
const searchPage = new SearchPage(page);
const specialCharsInput = '!@#$%^&*()_+-=[]{}|;:\'",.<>/?`~';
await searchPage.autoFillHasRelevantResults(specialCharsInput);
});
Then('the autofill dropdown should handle very long input strings', async ({page}) => {
const searchPage = new SearchPage(page);
const longInput = 'a'.repeat(256); // 256 characters long
await searchPage.autoFillHasRelevantResults(longInput);
});



Then('the {string} list should be sortable', async ({page}, arg: string) => {
var columns:string[] = getColumns(arg);
Copy link
Contributor

Choose a reason for hiding this comment

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

issue (code-quality): Use const or let instead of var. (avoid-using-var)

Explanation`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code). `let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than function-scoped.

From the Airbnb JavaScript Style Guide


Then('the {string} list should be sortable', async ({page}, arg: string) => {
var columns:string[] = getColumns(arg);
var id:string = getPaginationId(arg);
Copy link
Contributor

Choose a reason for hiding this comment

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

issue (code-quality): Use const or let instead of var. (avoid-using-var)

Explanation`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code). `let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than function-scoped.

From the Airbnb JavaScript Style Guide

});

Then('the user should be able to switch to next {string} items', async ({page}, arg: string) => {
var id:string = getPaginationId(arg);
Copy link
Contributor

Choose a reason for hiding this comment

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

issue (code-quality): Use const or let instead of var. (avoid-using-var)

Explanation`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code). `let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than function-scoped.

From the Airbnb JavaScript Style Guide

Then('the user should be able to increase pagination for the {string}', async ({page}, arg: string) => {
const info = getTableInfo(arg);
const table = new ToolbarTable(page,info[0]);
var id:string = getPaginationId(arg);
Copy link
Contributor

Choose a reason for hiding this comment

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

issue (code-quality): Use const or let instead of var. (avoid-using-var)

Explanation`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code). `let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than function-scoped.

From the Airbnb JavaScript Style Guide

Copy link
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.

@matejnesuta There are some useful scenarios being tested in this PR, but unfortunately I think it introduces a lot of redundancy and uses the helper classes that are now deprecated. Could you please move any new page-related functionality to appropriate classes in the pages directory (not the helpers directory) and re-factor the PR, so that it uses what is already defined there?

Given User is authenticated
And User is on the Search page

Scenario: User visits search page without filling anything
Copy link
Contributor

Choose a reason for hiding this comment

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

We definitely shouldn't hardcode numbers of SBOMs, packages etc. This would make the test suite annoying to maintain, as we add more SBOMs with new tests. I'm not sure if this scenario should be used at all, to be honest. The only way I can imagine this could be useful is if we did an API check for number of SBOMs, packages etc. and compared it with the numbers shown on the search page.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree. I asked about this before during our daily sync and I was told to hard-code it for now and then later re-write it using the API. The comparison is also not strict, meaning that there should be at least a given number of SBOMs, packages, etc, but a bigger number is also tolerated.

Copy link
Contributor

Choose a reason for hiding this comment

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

@matejnesuta Can you please parameterize these values with Examples section? something like this,

  Then a total number of "<Count1>" "<Label1>" should be visible in the tab
  And a total number of "<Count2>" "<Label2>" should be visible in the tab
  And a total number of "<Count3>" "<Label3>" should be visible in the tab
  And a total number of "<Count4>" "<Label4>" should be visible in the tab

Examples:
  | Count1 | Label1  | Count2 | Label2     | Count3 | Label3          | Count4 | Label4     |
  | 17     | SBOMs   | 5537   | Packages   | 29     | Vulnerabilities | 57     | Advisories |


Scenario Outline: User toggles the "<types>"list and manipulates the list
When User selects the Tab "<types>"
Then the "<types>" list should have specific filter set
Copy link
Contributor

Choose a reason for hiding this comment

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

In my opinion, the wording of this step is not clear. We could make this step more useful (and reusable), if the step was called something like Then the "<types>" list should have the <filter> filter set. This way it would be immediately visible what the filter is.

Scenario Outline: User toggles the "<types>"list and manipulates the list
When User selects the Tab "<types>"
Then the "<types>" list should have specific filter set
And the "<types>" list should be sortable
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably make sure each column is sortable. BUT I think that this may be redundant, as this is already being tested here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Though on second thought it may be a good idea to still test this, as BDD scenarios are likely to run separately from the tests written by Carlos, but the test step could re-use the methods defined in his page objects.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be fair, I am kind of inclined to rewriting this, so the test just checks relevant aria-sort label.

I am currently not sure, whether the sorting is performed on frontend or backend, burt I feel like it should be tested either with some sort of React component test, or an API test. I tried to test it using an E2E test, but I have not been able to make it not flake in the Github CI.

Copy link
Contributor

Choose a reason for hiding this comment

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

The methods verifySorting validates the frontend - It is capable of validating different data types (date, cve,..) as well.

@@ -0,0 +1,275 @@
import { SearchPage } from "../../helpers/SearchPage";
Copy link
Contributor

Choose a reason for hiding this comment

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

With Carlos' page object PR merged, we should probably consider these classes outdated and use the ones from that merge request if possible to avoid redundancy.

* This function returns table identifier and column, which contains link to the details page
* @param type Catogory of the data to get the table identifier and column
*/
function getTableInfo(type: string): [string, string] {
Copy link
Contributor

Choose a reason for hiding this comment

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

Helper functions should not be defined in the step definition file. Please use the methods defined for the page objects as mentioned above, if applicable, and if you have to extend them, please do it there.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree

"user starts typing a {string} in the search bar",
async ({ page }, searchText: string) => {
const searchPage = new SearchPage(page);
await searchPage.typeInSearchBox(searchText);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm pretty sure methods for something like this already exist in the repo. Again, please, let's avoid redundant code and use what can be reused.

@queria queria added the tests label Sep 25, 2025
Copy link
Contributor

@mrrajan mrrajan left a comment

Choose a reason for hiding this comment

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

@matejnesuta I regret for the late review - Please find my review comments. Additionally,

  • Capitalize the first word on the feature file
  • On the typescript methods and step definitions, use relevant naming instead of arg

Given User is authenticated
And User is on the Search page

Scenario: User visits search page without filling anything
Copy link
Contributor

Choose a reason for hiding this comment

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

@matejnesuta Can you please parameterize these values with Examples section? something like this,

  Then a total number of "<Count1>" "<Label1>" should be visible in the tab
  And a total number of "<Count2>" "<Label2>" should be visible in the tab
  And a total number of "<Count3>" "<Label3>" should be visible in the tab
  And a total number of "<Count4>" "<Label4>" should be visible in the tab

Examples:
  | Count1 | Label1  | Count2 | Label2     | Count3 | Label3          | Count4 | Label4     |
  | 17     | SBOMs   | 5537   | Packages   | 29     | Vulnerabilities | 57     | Advisories |

Scenario Outline: User toggles the "<types>"list and manipulates the list
When User selects the Tab "<types>"
Then the "<types>" list should have specific filter set
And the "<types>" list should be sortable
Copy link
Contributor

Choose a reason for hiding this comment

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

The methods verifySorting validates the frontend - It is capable of validating different data types (date, cve,..) as well.

Comment on lines +19 to +21
And the "<types>" list should be limited to 10 items
And the user should be able to switch to next "<types>" items
And the user should be able to increase pagination for the "<types>"
Copy link
Contributor

Choose a reason for hiding this comment

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

All these steps seems to Pagination related which can be verified with a single step definition using verifyPagination method under ToolbarTable.ts helper file.

And the user should be navigated to the specific "<type-instance>" page

Examples:
|type|types|type-instance|
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we have just type or types?

* This function returns table identifier and column, which contains link to the details page
* @param type Catogory of the data to get the table identifier and column
*/
function getTableInfo(type: string): [string, string] {
Copy link
Contributor

Choose a reason for hiding this comment

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

Agree

Comment on lines +226 to +236
"the user should be able to increase pagination for the {string}",
async ({ page }, arg: string) => {
const info = getTableInfo(arg);
const table = new ToolbarTable(page, info[0]);
var id: string = getPaginationId(arg);
const tableTopPagination = `xpath=//div[@id="${id}"]`;
await table.verifyPagination(`xpath=//div[@id="${id}"]`);
await table.goToFirstPage(tableTopPagination);
await table.selectPerPage(tableTopPagination, "20 per page");
await table.goToFirstPage(tableTopPagination);
await table.verifyTableHasUpToRows(20);
Copy link
Contributor

Choose a reason for hiding this comment

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

This step seems to be redundant as "the user should be able to switch to next {string} items" has table.verifyPagination() method to verify the same.

);
if (firstNonEmptyRowIndex !== -1) {
row = firstNonEmptyRowIndex;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I made a mistake on the looping which should be

 for (const data of dataRow) {
      if (data[index] !== ``) {

        break;
      } 
      row += 1;
}

Your version is better though!

Comment on lines +393 to +396
// } else if (isCVE) {
// const [cveYA, cveIA] = this.getCVE(valueA);
// const [cveYB, cveIB] = this.getCVE(valueB);
// compare = cveYA !== cveYB ? cveYA - cveYB : cveIA - cveIB;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this meant to be commented out? This validation is required for SBOM explorer vulnerabilities table sorting validation

Comment on lines +448 to +450
if (cvssScore === null && cvssString.includes("Unknown")) {
return 20;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please explain this block

Comment on lines +480 to +490
// Wait for PatternFly to finish DOM update
await this._page.waitForTimeout(50); // small buffer
await this._page.waitForFunction(
({ header, order }) => {
const th = Array.from(document.querySelectorAll("th")).find(th =>
th.textContent?.includes(header)
);
return th?.getAttribute("aria-sort") === order;
},
{ header: columnHeader, order: sortOrder }
);
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure this is required as it feels redundant to me.

 const sort = await headerElem.getAttribute(`aria-sort`);
      if (sort === sortOrder) {

Verifies the sorting to the desired order and we can include the timeout on the else condition.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants