chore(e2e): fix and unskip github discovery tests

[dzemanov]

Description

Fix and unskip github discovery tests.
Switches login from GitHub to Guest as there are more steps required for Github login, for example see github.spec.ts . This spec tests github auth flow, so no need to verify it works also in this test.

Which issue(s) does this PR fix

• Fixes https://redhat.atlassian.net/browse/RHDHBUGS-2576

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

[rhdh-qodo-merge]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. Repo listing not paginated ☑ 🐞 Bug ☼ Reliability

Description

GithubApi.getReposFromOrg fetches a single page from the org repos endpoint, and the test then
slices candidates to 5 entries, which can easily miss valid repos with catalog-info.yaml and cause
false failures. The repo already has a paginated GitHub helper, but this test path doesn’t use it.

Code

e2e-tests/playwright/e2e/github-discovery.spec.ts[R36-43]

    const organizationRepos = await githubApi.getReposFromOrg(testOrganization);
-    const reposNames: string[] = organizationRepos.map((repo) => repo["name"]);
-    const realComponents: string[] = reposNames.filter(
-      async (repo) =>
-        await githubApi.fileExistsOnRepo(
-          `${testOrganization}/${repo}`,
-          CATALOG_FILE,
+    const reposNames: string[] = (organizationRepos as Array<{ name?: string }>)
+      .map((repo) => repo.name)
+      .filter((name): name is string => typeof name === "string")
+      // filter for subset of organization repositories where the repository name matches the entity name
+      .filter((name) => name.startsWith("test-annotator"))
+      .slice(0, 5);
+

Relevance

⭐⭐ Medium

Pagination robustness is valid, but reviewers often reject broader E2E hardening/overengineering
changes; unclear priority here.

PR-#3831
PR-#4437
PR-#4549

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

getReposFromOrg returns the JSON body of a single GET to the org’s repos_url without page/per_page
handling. The updated test further reduces the search space with .slice(0, 5), which increases the
chance the test won’t find any repo containing catalog-info.yaml even if such repos exist
elsewhere in the org. The codebase already includes a recursive pagination helper, indicating
pagination is a known need.

e2e-tests/playwright/support/api/github.ts[22-25]
e2e-tests/playwright/support/api/github.ts[62-68]
e2e-tests/playwright/e2e/github-discovery.spec.ts[36-43]
e2e-tests/playwright/utils/api-helper.ts[44-65]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The test’s repo candidate selection is based on a non-paginated org repo listing plus `.slice(0, 5)`. This can miss valid repos and make the test intermittently fail.

### Issue Context
`GithubApi.getReposFromOrg()` currently performs a single GET to `repos_url` and returns `req.json()`; GitHub list endpoints are paginated, and this repo already includes a pagination helper (`APIHelper.getGithubPaginatedRequest`).

### Fix Focus Areas
- e2e-tests/playwright/support/api/github.ts[22-25]
- e2e-tests/playwright/support/api/github.ts[62-68]
- e2e-tests/playwright/e2e/github-discovery.spec.ts[36-55]

### Suggested fix
Implement one of:
- **Add pagination support to `GithubApi.getReposFromOrg`** (e.g., loop over `page` with `per_page=100` until empty, or reuse the existing pagination helper).
- **Progressive search strategy** in the test: iterate repos and stop once you’ve found N repos that contain `catalog-info.yaml` (with a max checked cap), instead of slicing before checking file existence.

This reduces false negatives while keeping API calls bounded.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Missing overlay migration note ☑ 📎 Requirement gap ⚙ Maintainability

Description

The unskipped GitHub discovery test changes do not include any explicit indication that this test is
a candidate for migration to rhdh-plugin-export-overlays, as required. The newly added hard-coded
repository-name filtering (test-annotator) also adds repository-specific coupling that may
complicate migration portability.

Code

e2e-tests/playwright/e2e/github-discovery.spec.ts[R31-42]

+  test(`Discover Organization's Catalog`, async ({
    catalogPage,
    githubApi,
    testOrganization,
  }) => {
    const organizationRepos = await githubApi.getReposFromOrg(testOrganization);
-    const reposNames: string[] = organizationRepos.map((repo) => repo["name"]);
-    const realComponents: string[] = reposNames.filter(
-      async (repo) =>
-        await githubApi.fileExistsOnRepo(
-          `${testOrganization}/${repo}`,
-          CATALOG_FILE,
+    const reposNames: string[] = (organizationRepos as Array<{ name?: string }>)
+      .map((repo) => repo.name)
+      .filter((name): name is string => typeof name === "string")
+      // filter for subset of organization repositories where the repository name matches the entity name
+      .filter((name) => name.startsWith("test-annotator"))
+      .slice(0, 5);

Relevance

⭐ Low

Team often rejects “add rationale/ticket note” comments in E2E code; migration-note requirement
likely not enforced.

PR-#4186
PR-#3994
PR-#4173

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 2 requires an explicit reference acknowledging this test as a migration candidate
and avoiding repository-specific assumptions that block migration. In the updated test block there
is no migration candidacy note/comment, and the newly introduced name.startsWith("test-annotator")
filter is a repo-specific assumption added by this PR.

Prepare GitHub Discovery test for potential migration to overlay repository
e2e-tests/playwright/e2e/github-discovery.spec.ts[31-42]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The updated `github-discovery.spec.ts` does not include an explicit note indicating the test is a candidate for migration to `https://github.com/redhat-developer/rhdh-plugin-export-overlays`, and the new hard-coded repo-name filtering (`test-annotator`) introduces repository-specific coupling that may hinder portability.

## Issue Context
PR Compliance ID 2 requires (a) an explicit migration-candidacy reference (comment/doc/PR note) and (b) avoiding new tight coupling that would make migration impractical.

## Fix Focus Areas
- e2e-tests/playwright/e2e/github-discovery.spec.ts[31-42]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Stateful repo-name dependency ☑ 🐞 Bug ☼ Reliability

Description

The discovery test only looks at repos starting with "test-annotator" and then asserts at least one
has catalog-info.yaml, so it can fail when those repos don’t exist in the org at runtime. In this
repo, the only producer of "test-annotator-*" repos also deletes them in afterAll, so the discovery
test becomes order/state-dependent instead of self-contained.

Code

e2e-tests/playwright/e2e/github-discovery.spec.ts[R37-57]

+    const reposNames: string[] = (organizationRepos as Array<{ name?: string }>)
+      .map((repo) => repo.name)
+      .filter((name): name is string => typeof name === "string")
+      // filter for subset of organization repositories where the repository name matches the entity name
+      .filter((name) => name.startsWith("test-annotator"))
+      .slice(0, 5);
+
+    const reposWithCatalogInfo: string[] = (
+      await Promise.all(
+        reposNames.map(async (repo) =>
+          (await githubApi.fileExistsOnRepo(
+            `${testOrganization}/${repo}`,
+            CATALOG_FILE,
+          ))
+            ? repo
+            : null,
        ),
-    );
+      )
+    ).filter((repo): repo is string => typeof repo === "string");
+
+    expect(reposWithCatalogInfo.length).toBeGreaterThan(0);

Relevance

⭐ Low

Hardcoded repo prefix likely intentional fixture for QE org; similar E2E brittleness concerns
frequently not acted on.

PR-#3831
PR-#4173
PR-#4341

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

github-discovery.spec.ts restricts candidates to a hardcoded prefix and requires at least one match.
The only local reference creating repos with that prefix is the annotator E2E, and it cleans them up
at the end; therefore these repos are not a stable fixture to depend on, making the discovery test
brittle across clean environments and test ordering.

e2e-tests/playwright/e2e/github-discovery.spec.ts[36-63]
e2e-tests/playwright/e2e/plugins/scaffolder-backend-module-annotator/annotator.spec.ts[24-35]
e2e-tests/playwright/e2e/plugins/scaffolder-backend-module-annotator/annotator.spec.ts[176-185]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`github-discovery.spec.ts` depends on repos named `test-annotator*` existing in the GitHub org, then hard-fails if none are found. Those repos are not a stable fixture (they’re dynamically created elsewhere and deleted in cleanup), so the discovery test becomes order/state-dependent and can fail in clean CI runs.

### Issue Context
The goal of this test is to validate GitHub discovery/catalog behavior, not to validate the presence of leftover repos in `janus-qe`.

### Fix Focus Areas
- e2e-tests/playwright/e2e/github-discovery.spec.ts[31-63]

### Suggested fix
Pick one of these approaches:
1) **Create a dedicated discovery repo in this test** (preferred for isolation):
  - In `beforeAll`, use existing helpers to create a repo in `JANUS_QE_ORG` with a `catalog-info.yaml` (e.g. `APIHelper.createGitHubRepoWithFile(...)`).
  - Wait for the entity to appear in the catalog and assert it’s visible.
  - In `afterAll`, delete the repo.

2) **Use a deterministic allow-list** of known repos guaranteed to exist and contain `catalog-info.yaml` (no hardcoded prefix, no reliance on other test suites).

In both cases, remove the `test-annotator` prefix assumption and avoid asserting on org state you don’t control.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[rhdh-qodo-merge]

Review Summary by Qodo

Fix and unskip GitHub discovery E2E tests

🧪 Tests 🐞 Bug fix

Walkthroughs

Description

• Remove test.fixme() and fix skipped GitHub discovery test
• Replace console.assert with Playwright expect assertions
• Improve repository filtering logic with proper TypeScript typing
• Add validation that catalog files exist before assertions

Diagram

flowchart LR
  A["Test Fixture Setup"] -->|"Remove fixme marker"| B["Active Test Execution"]
  B -->|"Filter repos by name"| C["Get repos with catalog"]
  C -->|"Validate results"| D["Assert visibility with expect"]
  E["console.assert"] -->|"Replace with"| F["Playwright expect"]

Loading

File Changes

1. e2e-tests/playwright/e2e/github-discovery.spec.ts 🧪 Tests +24/-14

Fix and enable GitHub discovery test with proper assertions

• Removed test.fixme() wrapper to enable previously skipped test
• Replaced console.assert with Playwright's expect assertion API
• Improved repository filtering with proper TypeScript type guards and filtering for test-annotator
 repos
• Refactored async filtering logic using Promise.all() to properly handle async file existence
 checks
• Added explicit validation that at least one repo with catalog info exists before iteration
• Changed loop from indexed to for-of for cleaner iteration over filtered results

e2e-tests/playwright/e2e/github-discovery.spec.ts

---

[github-actions]

Image was built and published successfully. It is available at:

• quay.io/rhdh-community/rhdh:pr-4626
• quay.io/rhdh-community/rhdh:pr-4626-77073da4

[github-actions]

Image was built and published successfully. It is available at:

• quay.io/rhdh-community/rhdh:pr-4626
• quay.io/rhdh-community/rhdh:pr-4626-1006ff0d

[github-actions]

The container image build and publish workflows were skipped (either due to [skip-build] tag or no relevant changes with existing image).

[github-actions]

Image was built and published successfully. It is available at:

• quay.io/rhdh-community/rhdh:pr-4626
• quay.io/rhdh-community/rhdh:pr-4626-790789e0

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[github-actions]

Image was built and published successfully. It is available at:

• quay.io/rhdh-community/rhdh:pr-4626
• quay.io/rhdh-community/rhdh:pr-4626-b34d782a

[openshift-ci]

@dzemanov: 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-ocp-helm	b34d782	link	true	/test e2e-ocp-helm

Full PR test history. Your PR dashboard.

Details

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.