Skip to content

feat(e2e): add tests for GitHub plugins (PR statistics & security insights) #2998

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 15 commits into
base: main
Choose a base branch
from

Conversation

guyoron1
Copy link

Description

This PR adds end-to-end tests for two GitHub-related plugins in RHDH:

  1. GitHub Pull Requests Statistics plugin
  2. GitHub Security Insights plugin

Key changes:

  • Created new test files:
    • e2e-tests/playwright/e2e/plugins/github-pull-requests.spec.ts: Tests PR statistics functionality including metrics like average PR time, merge ratio, etc.
    • e2e-tests/playwright/e2e/plugins/github-security-insights.spec.ts: Tests security insights functionality including Dependabot alerts severity levels
  • Enhanced common.ts with improved GitHub authentication:
    • Added githubLoginPopUpModal function to handle GitHub login in a popup context
    • Implemented robust 2FA handling
    • Prevents context switching issues by maintaining the popup state

The new authentication approach improves test reliability by handling the GitHub login flow in a popup modal without navigating away from the main application context.

Which issue(s) does this PR fix

  • Adds test coverage for GitHub plugins (no specific issue)

PR acceptance criteria

  • GitHub Actions are completed and successful
  • E2E Tests are updated and passing
    • Added new E2E tests for GitHub PR statistics plugin
    • Added new E2E tests for GitHub Security Insights plugin
    • All tests pass locally with proper environment variables set
  • Documentation is updated if necessary (not required - test additions only)

How to test changes / Special notes to the reviewer

To run the tests locally:

  1. Set required environment variables:
export BASE_URL="your-rhdh-url"
export GH_USER_ID="your-github-username"
export GH_USER_PASS="your-github-password"
export GH_2FA_SECRET="your-github-2fa-secret"
  1. Run specific test files:
cd e2e-tests
npx playwright test playwright/e2e/plugins/github-pull-requests.spec.ts --project=showcase --headed
npx playwright test playwright/e2e/plugins/github-security-insights.spec.ts --project=showcase --headed

The tests verify:

  • GitHub PR Statistics:

    • Average Time of PR Until Merge
    • Merged To Closed Ratio
    • Average Size of PR
    • Average Changed Files of PR
    • Average Coding Time of PR
  • Security Insights:

    • Critical severity alerts
    • High severity alerts
    • Medium severity alerts
    • Low severity alerts

Copy link

openshift-ci bot commented Jun 17, 2025

Hi @guyoron1. 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.

Copy link
Contributor

@gustavolira
Copy link
Member

/ok-to-test

my_changes.diff Outdated
@@ -0,0 +1,66 @@
diff --git a/.ibm/pipelines/value_files/values_showcase.yaml b/.ibm/pipelines/value_files/values_showcase.yaml
Copy link
Member

Choose a reason for hiding this comment

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

Why did you commit this file?

Comment on lines 210 to 229
await githubPage.fill('input[name="login"]', username);
await githubPage.fill('input[name="password"]', password);
Copy link
Member

Choose a reason for hiding this comment

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

I think you can reuse uiHelper.fillTextInputByLabel for all those page.fill

await uiHelper.openSidebar("Catalog");
// Step 3: Search and click 'Backstage Showcase'
await page.fill('input[placeholder="Search"]', repoName);
await page.waitForSelector('a:has-text("Backstage Showcase")', { timeout: 20000 });
Copy link
Member

Choose a reason for hiding this comment

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

await catalog.goToByName("Backstage Showcase");

Choose a reason for hiding this comment

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

export class CatalogHelper {
private page: Page;
private uiHelper: UIhelper;

constructor(page: Page) {
this.page = page;
this.uiHelper = new UIhelper(page);
}

async goToByName(name: string): Promise {
await this.uiHelper.openSidebar("Catalog");
await this.page.fill('input[placeholder="Search"]', name);
await this.page.waitForSelector(a:has-text("${name}"), { timeout: 20000 });
await this.uiHelper.clickLink(name);
}
}

and then it would be easier to maintain and much clearer

await page.fill('input[placeholder="Search"]', repoName);
await page.waitForSelector('a:has-text("Backstage Showcase")', { timeout: 20000 });
await uiHelper.clickLink(repoName);
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.

don't use await page.waitForLoadState("networkidle");

// Step 5: Click 'Sign in' inside the PR statistics card
await uiHelper.clickBtnInCard('GitHub Pull Requests Statistics', 'Sign in', true);
// Wait for the login modal to appear
const modalLoginButton = page.locator('button:has-text("Log in")');
Copy link
Member

Choose a reason for hiding this comment

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

uiHelper.isBtnVisible
uiHelper.getButtonSelector

]);
await githubPage.waitForLoadState();
await githubPage.waitForSelector('input[name="login"]', { timeout: 30000 });
await githubPage.fill('input[name="login"]', username);
Copy link
Member

Choose a reason for hiding this comment

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

uiHelper.fillTextInputByLabel


// Handle 2FA
let otpSelector = null;
if (await githubPage.isVisible('input[name="otp"]', { timeout: 10000 }).catch(() => false)) {
Copy link
Member

Choose a reason for hiding this comment

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

maybe you can use uiHelper.isElementVisible


async GetChildGroupsDisplayed(): Promise<string[]> {
await this.page.waitForSelector("p:has-text('Child Groups')");
const parent = await this.page
Copy link
Member

Choose a reason for hiding this comment

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

private async getGroupLinksByLabel(label: string): Promise<string[]> {
  const section = this.page.locator(`p:has-text("${label}")`).first();
  await section.waitFor({ state: "visible" });

  return await section.locator("..").locator("a").allInnerTexts();
}

async getParentGroupsDisplayed(): Promise<string[]> {
  return await this.getGroupLinksByLabel("Parent Group");
}

async getChildGroupsDisplayed(): Promise<string[]> {
  return await this.getGroupLinksByLabel("Child Groups");
}

.filter((u) => u.spec.profile && u.spec.profile.displayName)
.map((u) => u.spec.profile.displayName);
LOGGER.info(
`Checking ${JSON.stringify(catalogUsersDisplayNames)} contains users ${JSON.stringify(users)}`,
Copy link
Member

Choose a reason for hiding this comment

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

type EntityWithDisplay = { spec?: { profile?: { displayName?: string } } };

export class CatalogVerifier {
  private api: APIHelper;

  constructor(token: string) {
    this.api = new APIHelper();
    this.api.UseStaticToken(token);
  }

  async assertUsersInCatalog(expected: string[]): Promise<void> {
    await this.assertEntities(
      () => this.api.getAllCatalogUsersFromAPI(),
      expected,
      'users',
    );
  }

  async assertGroupsInCatalog(expected: string[]): Promise<void> {
    await this.assertEntities(
      () => this.api.getAllCatalogGroupsFromAPI(),
      expected,
      'groups',
    );
  }

  private async assertEntities(
    fetch: () => Promise<{ items?: EntityWithDisplay[] }>,
    expected: string[],
    label: string,
  ): Promise<void> {
    const { items = [] } = await fetch();

    expect(items.length).toBeGreaterThan(0);

    const displayNames = items
      .flatMap(e => e.spec?.profile?.displayName ?? []);

    const catalogSet = new Set(displayNames);
    const missing = expected.filter(name => !catalogSet.has(name));

    LOGGER.info(
      `Catalog ${label}: [${displayNames.join(', ')}] – expecting [${expected.join(', ')}]`,
    );

    expect(missing, `Missing ${label}: ${missing.join(', ')}`).toHaveLength(0);
  }
}

const secret = secrets[userid];
if (!secret) {
throw new Error("Invalid User ID");
async githubLoginPopUpModal(context, username, password, otpSecret) {
Copy link
Member

Choose a reason for hiding this comment

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

async function githubLoginPopUpModal(
context: BrowserContext,
username: string,
password: string,
otpSecret: string
): Promise {
const [githubPage] = await Promise.all([
context.waitForEvent('page'),
]);

await githubPage.waitForLoadState('domcontentloaded');
await githubPage.fill('input[name="login"]', username);
await githubPage.fill('input[name="password"]', password);
await githubPage.click('button[type="submit"], input[type="submit"]');

const otpSelector = await findOtpSelector(githubPage);

if (otpSelector) {
if (githubPage.isClosed()) return;

await githubPage.waitForSelector(otpSelector, { timeout: 10000 });
if (githubPage.isClosed()) return;

const otp = authenticator.generate(otpSecret);
await githubPage.fill(otpSelector, otp);

if (githubPage.isClosed()) return;

await Promise.race([
  githubPage.waitForEvent('close', { timeout: 20000 }),
  githubPage.click('button[type="submit"], input[type="submit"]'),
]);

} else {
await githubPage.waitForEvent('close', { timeout: 20000 });
}
}

async function findOtpSelector(page: Page): Promise<string | null> {
const selectors = ['input[name="otp"]', '#app_totp'];

for (const selector of selectors) {
try {
if (await page.isVisible(selector, { timeout: 5000 })) {
return selector;
}
} catch {
// Ignore visibility timeout errors
}
}
return null;
}

Copy link

openshift-ci bot commented Jun 17, 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 gustavolira 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

@@ -213,6 +213,9 @@ global:
disabled: false
- package: ./dynamic-plugins/dist/red-hat-developer-hub-backstage-plugin-global-floating-action-button
disabled: false
# Enable Security Insights plugin
Copy link
Member

Choose a reason for hiding this comment

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

nit: Please remove this comment. It's clear that it's enabling security insights plugin based on the name of the imported plugin name

"prettier": "3.5.3",
"typescript": "5.8.3"
},
"dependencies": {
"@azure/identity": "4.9.1",
"@backstage/catalog-model": "^1.7.4",
Copy link
Member

Choose a reason for hiding this comment

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

I think we don't need to import this library. You are importing this lib only to type a field as far as I could see


await common.loginAsKeycloakUser(process.env.GH_USER_ID, process.env.GH_USER_PASS);
await uiHelper.openSidebar("Catalog");
await page.fill('input[placeholder="Search"]', repoName);
Copy link
Member

Choose a reason for hiding this comment

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

you can use uiHelper.searchInputPlaceholder. Please replace all occurrences with it.

await githubPage.waitForLoadState('domcontentloaded');

await githubPage.getByLabel('Username or email address').fill(username);
await githubPage.getByLabel('Password').fill(password);
Copy link
Member

Choose a reason for hiding this comment

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

uiHelper.fillTextInputByLabel replace all occurrences with it


await githubPage.getByLabel('Username or email address').fill(username);
await githubPage.getByLabel('Password').fill(password);
await githubPage.click('button[type="submit"], input[type="submit"]');
Copy link
Member

Choose a reason for hiding this comment

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

uiHelper.clickButton replace all

Copy link
Member

Choose a reason for hiding this comment

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

Please update this one

Copy link
Contributor

@guyoron1 guyoron1 force-pushed the pr-statistics-security-insights-branch branch from 1925ab6 to dcc2cf2 Compare June 23, 2025 13:44
@guyoron1 guyoron1 changed the title Pr statistics security insights branch feat(e2e): add tests for github plugins - pr statistics and security insights Jun 23, 2025
const [githubPage] = await Promise.all([
context.waitForEvent('page'),
]);
await githubPage.waitForLoadState('domcontentloaded');
Copy link
Member

Choose a reason for hiding this comment

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

it's better to use specific locators instead of load state.
for example: await page.waitForSelector('text=something');


await githubPage.getByLabel('Username or email address').fill(username);
await githubPage.getByLabel('Password').fill(password);
await githubPage.click('button[type="submit"], input[type="submit"]');
Copy link
Member

Choose a reason for hiding this comment

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

Please update this one

if (githubPage.isClosed()) return;
await Promise.race([
githubPage.waitForEvent('close', { timeout: 20000 }),
githubPage.click('button[type="submit"], input[type="submit"]'),
Copy link
Member

Choose a reason for hiding this comment

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

uiHelper.clickButton

for (const selector of selectors) {
try {
if (await page.isVisible(selector, { timeout: 5000 })) {
return selector;
Copy link
Member

Choose a reason for hiding this comment

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

The silent catch could lead to false positives. If a timeout or other issue occurs, it gets ignored and the function may return null even when something went wrong. Might be worth adding logging or refining the error handling.

/**
 * Looks for an OTP input field and returns its selector.
 * Throws if no field becomes visible within the overall timeout.
 */
async function findOtpSelector(page: Page): Promise<string> {
  const selectors = ['input[name="otp"]', '#app_totp'];

  for (const sel of selectors) {
    try {
      // Wait until the element is visible (up to 10 s)
      await page.locator(sel).waitFor({ state: 'visible', timeout: 10_000 });
      return sel;                       // Found a visible OTP field
    } catch (err) {
      // Timeout for this selector — log and try the next one
      console.debug(`Selector ${sel} not visible yet, continuing…`);
    }
  }

  // None of the selectors became visible in time
  throw new Error('OTP field not found on the page within 10 s');
}

Copy link
Contributor

Copy link
Contributor

@gustavolira
Copy link
Member

/test e2e-tests

@guyoron1 guyoron1 force-pushed the pr-statistics-security-insights-branch branch from 125e042 to 82bc0bf Compare June 30, 2025 12:13
@guyoron1 guyoron1 changed the title feat(e2e): add tests for github plugins - pr statistics and security insights feat(e2e): add tests for GitHub plugins (PR statistics & security insights) Jun 30, 2025
Copy link
Contributor

@gustavolira
Copy link
Member

/ok-to-test

@gustavolira
Copy link
Member

/test e2e-tests

@gustavolira
Copy link
Member

/test e2e-tests

Copy link
Contributor

@gustavolira
Copy link
Member

/test e2e-tests

@guyoron1 guyoron1 force-pushed the pr-statistics-security-insights-branch branch from 940639a to f6f4286 Compare July 13, 2025 15:43
Copy link
Contributor

@gustavolira
Copy link
Member

/test e2e-tests

@guyoron1 guyoron1 force-pushed the pr-statistics-security-insights-branch branch from 42f4517 to f6f4286 Compare July 15, 2025 11:45
@guyoron1 guyoron1 force-pushed the pr-statistics-security-insights-branch branch from 6d3bcf0 to 1b0406c Compare July 15, 2025 11:55
Copy link
Contributor

Copy link
Contributor

Copy link

openshift-ci bot commented Jul 16, 2025

@guyoron1: The following tests 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-sealight-helm-nightly e47da43 link false /test e2e-tests-sealight-helm-nightly
ci/prow/e2e-tests 2c59fdc 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.

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