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

---

3. 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

---

[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

[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

[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]

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

[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-84d30816

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.56%. Comparing base (1cf3ae8) to head (48ab421).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #4626       +/-   ##
===========================================
+ Coverage   40.98%   69.56%   +28.58%     
===========================================
  Files         119      109       -10     
  Lines        2223     4705     +2482     
  Branches      561      510       -51     
===========================================
+ Hits          911     3273     +2362     
- Misses       1306     1432      +126     
+ Partials        6        0        -6     

Flag	Coverage Δ
install-dynamic-plugins	92.44% <ø> (?)
rhdh	38.75% <ø> (-2.23%)	⬇️

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1cf3ae8...48ab421. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[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-a2198bb4

[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-48ab4213

[PatAKnight]

LGTM