Skip to content

test(e2e): add Github Insights plugin tests #2818

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

natifridman
Copy link

@natifridman natifridman commented Apr 29, 2025

Description

Add Github Insights plugin tests
https://github.com/RoadieHQ/roadie-backstage-plugins/tree/main/plugins/frontend/backstage-plugin-github-insights

Which issue(s) does this PR fix

PR acceptance criteria

Please make sure that the following steps are complete:

  • GitHub Actions are completed and successful
  • Unit Tests are updated and passing
  • E2E Tests are updated and passing
  • Documentation is updated if necessary (requirement for new features)
  • Add a screenshot if the change is UX/UI related

How to test changes / Special notes to the reviewer

@openshift-ci openshift-ci bot requested review from albarbaro and zdrapela April 29, 2025 09:18
Copy link

openshift-ci bot commented Apr 29, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign albarbaro for approval. For more information see the Code Review Process.

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link

openshift-ci bot commented Apr 29, 2025

Hi @natifridman. 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 /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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.

@natifridman natifridman changed the title test(e2e): Add Github Insights plugin tests test(e2e): add Github Insights plugin tests Apr 29, 2025
Copy link
Contributor

@natifridman
Copy link
Author

/cc @subhashkhileri @gustavolira

Copy link
Contributor

github-actions bot commented May 8, 2025

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.

@github-actions github-actions bot added the Stale label May 8, 2025
@gustavolira
Copy link
Member

/ok-to-test

import { Common } from "../../../utils/common";
import { UI_HELPER_ELEMENTS } from "../../../support/pageObjects/global-obj";

// Test to verify if the GitHub Insights plugin is installed - runs independently
Copy link
Member

Choose a reason for hiding this comment

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

This comment has the same text of the test description.


const uiHelper = new UIhelper(page);

// Navigate to the Administration in the sidebar
Copy link
Member

Choose a reason for hiding this comment

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

Excessive Comments:
While comments are useful, some explain actions that are already obvious from the function names.


// Wait for the page to load
await page.waitForLoadState("networkidle");

Copy link
Member

Choose a reason for hiding this comment

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

Move to a function into UiHelper

await uiHelper.openSidebar("Extensions");

// Wait for the page to load
await page.waitForLoadState("networkidle");
Copy link
Member

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="something"]') to ensure tests are more reliable and maintainable.

Comment on lines 55 to 57
await uiHelper.openSidebar("Catalog");
await uiHelper.selectMuiBox("Kind", "Component");
await uiHelper.clickByDataTestId("user-picker-all");
Copy link
Member

Choose a reason for hiding this comment

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

There's a function on UiHelper openCatalogSidebar you can reuse it
https://github.com/redhat-developer/rhdh/blob/main/e2e-tests/playwright/utils/ui-helper.ts#L207

await uiHelper.clickLink(repoWithGitHubInsights);

// Wait for entity page to load
await page.waitForLoadState("networkidle");
Copy link
Member

Choose a reason for hiding this comment

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

Same issue

Comment on lines 88 to 89
await uiHelper.verifyTextinCard("Compliance report", "License");
await uiHelper.verifyTextinCard("Compliance report", "Apache License");
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
await uiHelper.verifyTextinCard("Compliance report", "License");
await uiHelper.verifyTextinCard("Compliance report", "Apache License");
await uiHelper.verifyMultipleTextsInCard("Compliance report", ["License", "Apache License"]);

Copy link
Contributor

@natifridman natifridman requested a review from gustavolira May 13, 2025 08:50
Copy link
Contributor

@gustavolira
Copy link
Member

/test e2e-tests-nightly-auth-providers

Copy link

openshift-ci bot commented May 17, 2025

@gustavolira: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test e2e-tests

The following commands are available to trigger optional jobs:

/test cleanup-mapt-destroy-orphaned-aks-clusters
/test e2e-tests-aks-helm-nightly
/test e2e-tests-aks-operator-nightly
/test e2e-tests-gke-helm-nightly
/test e2e-tests-gke-operator-nightly
/test e2e-tests-nightly
/test e2e-tests-operator-nightly
/test e2e-tests-osd-gcp-helm-nightly
/test e2e-tests-osd-gcp-operator-nightly
/test e2e-tests-upgrade-nightly

Use /test all to run the following jobs that were automatically triggered:

pull-ci-redhat-developer-rhdh-main-e2e-tests

In response to this:

/test e2e-tests-nightly-auth-providers

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.

@natifridman natifridman force-pushed the github-insight branch 2 times, most recently from 3e37596 to 37aba26 Compare May 21, 2025 08:16
testMatch: ["**/playwright/e2e/authProviders/*.spec.ts"],
testMatch: [
"**/playwright/e2e/authProviders/*.spec.ts",
"**/playwright/e2e/plugins/github-insights/*.spec.ts",
Copy link
Member

@albarbaro albarbaro May 27, 2025

Choose a reason for hiding this comment

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

The auth-providers job is not the best place to run any GitHub plugin-related test; this job is going to be refactored soon and will not include any plugin test. Also, just adding the files in testMatch list is not enough to run them in this job, as they do not run against a single RHDH instance, but multiple ones are created and deleted dynamically during the tests execution.
I would recommend to setup the Github integration and the Github auth provider in the e2e main job configuration files and run the plugins tests along with the other plugins tests.
I'm going to create the OAuth for you and provide the necessary changes to update the configuration.

Copy link
Author

Choose a reason for hiding this comment

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

Hi @albarbaro, any update on this?

Copy link
Member

Choose a reason for hiding this comment

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

I have created the Oauth client. You need to update the configuration with following:

  • load the new environment variable from vault by adding here these lines
GITHUB_OAUTH_APP_ID=$(cat /tmp/secrets/GITHUB_OAUTH_APP_ID)
GITHUB_OAUTH_APP_SECRET=$(cat /tmp/secrets/GITHUB_OAUTH_APP_SECRET)
  • update the app-config to add the github auth provider by adding this config in here
auth:
    github:
      production:
        clientSecret: ${GITHUB_OAUTH_APP_SECRET}
        clientId: ${GITHUB_OAUTH_APP_ID}
        callbackUrl: ${RHDH_BASE_URL}/api/auth/github/handler/frame
  • the github-org plugin is apparently already enabled: you might want to adjust the sync frequency if the users are not ingested immediately
  • move **/playwright/e2e/plugins/github-insights/*.spec.ts out of the showcase-auth-providers playwright project and place them in the showcase one

The main configuration uses RHBK as the default signIn page: this means that your tests might need to be updated to login from the settings page if interactive login is required (but this is plugin specific).
cc @subhashkhileri @gustavolira please double check or raise any concern/error if you spot some.

@natifridman natifridman force-pushed the github-insight branch 2 times, most recently from 73b1c53 to e929f9d Compare June 5, 2025 06:32
Copy link
Contributor

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.

@github-actions github-actions bot added the Stale label Jun 14, 2025
@natifridman
Copy link
Author

Comment to remove stale

@natifridman
Copy link
Author

/remove stale

@github-actions github-actions bot removed the Stale label Jun 21, 2025
Copy link
Contributor

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.

Copy link
Contributor

@github-actions github-actions bot removed the Stale label Jun 30, 2025
@@ -104,6 +104,11 @@ auth:
signIn:
resolvers:
- resolver: emailLocalPartMatchingUserEntityName
github:
Copy link
Member

Choose a reason for hiding this comment

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

@@ -17,6 +17,8 @@ data:
GITHUB_URL: $GITHUB_URL
GITHUB_ORG: $GITHUB_ORG
GITHUB_ORG_2: $GITHUB_ORG_2
GITHUB_OAUTH_APP_SECRET: $GITHUB_OAUTH_APP_SECRET
Copy link
Member

@subhashkhileri subhashkhileri Jun 30, 2025

Choose a reason for hiding this comment

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

GITHUB_OAUTH secrete and app ID is already present in secrets

@@ -57,6 +57,9 @@ QE_USER5_PASS=$(cat /tmp/secrets/QE_USER5_PASS)
QE_USER6_ID=$(cat /tmp/secrets/QE_USER6_ID)
QE_USER6_PASS=$(cat /tmp/secrets/QE_USER6_PASS)

GITHUB_OAUTH_APP_ID=$(cat /tmp/secrets/GITHUB_OAUTH_APP_ID)
Copy link
Member

Choose a reason for hiding this comment

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

GITHUB_OAUTH secrete and app ID is already present in env_variables.sh

* Navigates to the Extensions page in the Administration section
* and selects the Installed tab
*/
async navigateToExtensions() {
Copy link
Member

Choose a reason for hiding this comment

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

there is already one method for this in PR #3013

please check the quoted comments "This method is doing more than just navigation. would it make sense to move other clicks in test itself so in future someone want to use this method other than installed Tab they can use it."

* @param texts Array of strings or RegExp to verify in the card
* @param exact Whether to match exact text
*/
async verifyMultipleTextsInCard(
Copy link
Member

Choose a reason for hiding this comment

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

@natifridman accepting [] in verifyTextinCard is fine without a need for another method ``verifyMultipleTextsInCard`


// Click on the repo link and wait for entity page heading to appear
await uiHelper.clickLink(repoWithGitHubInsights);
await page.waitForSelector("h1", { state: "visible" });
Copy link
Member

Choose a reason for hiding this comment

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

we have waitForTitle on UIHelper

Copy link
Member

Choose a reason for hiding this comment

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

Hi @natifridman can you please update this one?

Copy link
Contributor

github-actions bot commented Jul 8, 2025

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.

@github-actions github-actions bot added the Stale label Jul 8, 2025
Copy link
Contributor

Copy link

openshift-ci bot commented Jul 10, 2025

@natifridman: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-tests dc4458c link true /test e2e-tests

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.

@github-actions github-actions bot removed the Stale label Jul 11, 2025
Copy link
Contributor

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.

@github-actions github-actions bot added the Stale label Jul 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants