-
Notifications
You must be signed in to change notification settings - Fork 197
test(e2e): add Github actions plugin tests #2838
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test(e2e): add Github actions plugin tests #2838
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Hi @prabinovRedhat. Thanks for your PR. I'm waiting for a redhat-developer member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
6a6c876
to
a113db5
Compare
The image is available at: |
/ok-to-test |
/ok-to-test |
await page.waitForSelector('table, div[role="grid"], [data-testid="plugins-table"]', { timeout: 30000 }); | ||
await page.waitForTimeout(3000); // Add a delay to ensure the table is fully rendered |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using page.waitForLoadState('networkidle') and page.waitForLoadState('domcontentloaded') is not recommended for modern web applications. These methods rely on global states that can be unreliable in dynamic applications with ongoing network requests (e.g., WebSockets or API polling). Instead, it’s better to wait for specific elements or components that indicate the page is fully loaded and interactive. For example, use
page.waitForSelector('[data-testid="plugins-table"]')
to ensure tests are more reliable and maintainable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
console.log(`Searching for plugin with term: ${searchTerm}`); | ||
|
||
// Wait for search results | ||
await page.waitForTimeout(2000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using await page.waitForTimeout(2000); is generally not a best practice as it introduces arbitrary delays that can make tests slower and less reliable. Instead, it’s better to wait for specific elements or conditions that indicate the page or component is ready. For example:
await page.waitForSelector('[data-testid="expected-element"]', { timeout: 10000 });
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
await page.waitForLoadState('domcontentloaded', { timeout: 60000 }); | ||
await page.waitForLoadState('networkidle', { timeout: 60000 }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
b2e4e67
to
4c5d838
Compare
@gustavolira Please take a look again, after my fixes |
The image is available at: |
|
||
test("Verify backstage-community-plugin-github-actions in Extensions page", async ({ page }) => { | ||
// Navigate to Extensions page through Administration | ||
await uiHelper.openSidebarButton("Administration"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @prabinovRedhat can you please take this change in consideration
https://redhat-internal.slack.com/archives/C08JCP2F59B/p1746796660780629
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, when Shelly's or Nati's PR will be merged I'll use their function, is it ok?
@gustavolira
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Except of this comment, I fixed all the rest
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me know if it looks ok for you?
|
||
test.beforeEach(async ({ page }) => { | ||
// Increase default timeout to account for slower responses | ||
test.setTimeout(180000); // 3 minutes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can remove this timeout
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
await expect(nameCell).toContainText("github-actions"); | ||
|
||
// Verify plugin status (Enabled and Preinstalled should both be "Yes") | ||
const enabledCell = pluginRow.locator('td').nth(2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please check UiHelper.ts? I think there's some methods which you can reuse for it, for example verifyPluginRow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
await uiHelper.openSidebar("Catalog"); | ||
await uiHelper.selectMuiBox("Kind", "Component"); | ||
await uiHelper.clickByDataTestId("user-picker-all"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use UiHelper.openCatalogSidebar function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
8044de6
to
056d7cc
Compare
The image is available at: |
/test e2e-tests |
await uiHelper.searchInputPlaceholder(searchTerm); | ||
console.log(`Searching for plugin with term: ${searchTerm}`); | ||
|
||
// Use UIHelper to verify the plugin row instead of manual cell checking |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to keep comments that were used as instruction for AI in the code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you're right! I missed this one, I'll fix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed all comments
056d7cc
to
c6b234e
Compare
The image is available at: |
c6b234e
to
b586d39
Compare
b586d39
to
a77792d
Compare
Assisted-by: Cursor
a77792d
to
8353660
Compare
@prabinovRedhat: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
The image is available at: |
This PR is stale because it has been open 7 days with no activity. Remove stale label or comment or this will be closed in 21 days. |
My PR was closed, I didn't succeed to re-open it, so I opened a new one - #3012 |
Description
Add Github actions plugin tests
Plugin reference https://www.npmjs.com/package/@backstage-community/plugin-github-actions
Which issue(s) does this PR fix
Fixes RHIDP-6710
PR acceptance criteria
Please make sure that the following steps are complete:
How to test changes / Special notes to the reviewer