Fix the test coverage with Copilot CLI

[lukaszgryglicki]

This is a pure AI fix - I was testing Copilot CLI to fix intermittent issues on cypress E2E test coverage.

Then I checked this myself and all fixes look reasonable, hence this PR...

Signed-off-by: Lukasz Gryglicki lgryglicki@cncf.io

Assisted by OpenAI

Assisted by GitHub Copilot

[coderabbitai]

Walkthrough

Updates Cypress test loops in v4 signatures e2e tests: changes loop bounds from <= to <, introduces found flags, adds explicit failure when items aren’t found, and breaks early on match while keeping existing assertions.

Changes

Cohort / File(s)	Summary of Changes
Cypress v4 signatures tests tests/functional/cypress/e2e/v4/signatures.cy.ts	Replaced i <= length with i < length; added found flags; added explicit post-loop failure when target not found; corrected else-if accessing out-of-bounds; added early breaks on match; retained per-item assertions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title Check	❓ Inconclusive	The title “Fix the test coverage with Copilot CLI” is overly generic and focuses on the AI tool used rather than summarizing the core changes to the Cypress test logic, such as correcting loop bounds and adding explicit failure paths; it does not clearly convey the substantive modifications made to prevent out-of-bounds access in the tests.	Please update the title to clearly reflect the main change, for example “Improve Cypress E2E loop bounds and explicit failure handling” so reviewers immediately understand the purpose of the adjustments.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	The description explains that the PR addresses intermittent Cypress E2E test coverage issues by applying AI-assisted fixes and confirms the author’s review of those changes, which directly relates to the adjustments in test loops and validation logic.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch unicron-4762-cypress-apis-test-coverage-12

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull Request Overview

This PR fixes critical array indexing bugs in Cypress E2E tests that were causing intermittent test failures. The changes prevent array out-of-bounds access and improve test logic for validating approval lists.

• Fixed off-by-one errors in for-loop conditions across multiple test cases
• Replaced flawed conditional logic with proper found/not-found tracking
• Improved error messages for failed approval list validations

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (10)

tests/functional/cypress/e2e/v4/signatures.cy.ts (10)

470-472: Bounds fix looks good; simplify assertion.

Use array-guard + not.include to avoid manual iteration.

-        for (let i = 0; i < list.length; i++) {
-          expect(list[i]).to.not.equal(emailApprovalList);
-        }
+        expect(list, 'emailApprovalList').to.be.an('array');
+        expect(list).to.not.include(emailApprovalList);

---

518-520: Same here: prefer not.include over per-item loop.

Keeps intent while reducing noise.

-        for (let i = 0; i < list.length; i++) {
-          expect(list[i]).to.not.equal(gitOrgApprovalList);
-        }
+        expect(list, 'githubOrgApprovalList').to.be.an('array');
+        expect(list).to.not.include(gitOrgApprovalList);

---

541-552: Replace loop + flag with direct inclusion assertion.

Shorter, clearer, and consistent with other tests using include.

-      let found = false;
-      for (let i = 0; i < list.length; i++) {
-        if (list[i] === gitUsernameApprovalList) {
-          expect(list[i]).to.eql(gitUsernameApprovalList);
-          found = true;
-          break;
-        }
-      }
-      if (!found) {
-        expect.fail('GitHub Username not found in approval list');
-      }
+      expect(list, 'githubUsernameApprovalList').to.be.an('array');
+      expect(list).to.include(gitUsernameApprovalList);

---

572-574: Use not.include for removal check.

Removes the loop and keeps failure messaging readable.

-        for (let i = 0; i < list.length; i++) {
-          expect(list[i]).to.not.equal(gitUsernameApprovalList);
-        }
+        expect(list, 'githubUsernameApprovalList').to.be.an('array');
+        expect(list).to.not.include(gitUsernameApprovalList);

---

595-606: Apply the same inclusion pattern for GitLab usernames.

Aligns with other add-list tests.

-      let found = false;
-      for (let i = 0; i < list.length; i++) {
-        if (list[i] === gitUsernameApprovalList) {
-          expect(list[i]).to.eql(gitUsernameApprovalList);
-          found = true;
-          break;
-        }
-      }
-      if (!found) {
-        expect.fail('GitLab Username not found in approval list');
-      }
+      expect(list, 'gitlabUsernameApprovalList').to.be.an('array');
+      expect(list).to.include(gitUsernameApprovalList);

---

626-628: Compact the removal assertion.

Use not.include with an array guard.

-        for (let i = 0; i < list.length; i++) {
-          expect(list[i]).to.not.equal(gitUsernameApprovalList);
-        }
+        expect(list, 'gitlabUsernameApprovalList').to.be.an('array');
+        expect(list).to.not.include(gitUsernameApprovalList);

---

649-660: Use inclusion assertion for GitLab org add case.

Mirror the simpler pattern used elsewhere.

-      let found = false;
-      for (let i = 0; i < list.length; i++) {
-        if (list[i] === gitLabOrgApprovalList) {
-          expect(list[i]).to.eql(gitLabOrgApprovalList);
-          found = true;
-          break;
-        }
-      }
-      if (!found) {
-        expect.fail('GitLab Org not found in approval list');
-      }
+      expect(list, 'gitlabOrgApprovalList').to.be.an('array');
+      expect(list).to.include(gitLabOrgApprovalList);

---

680-682: Simplify removal org check.

Use not.include for clarity and consistency.

-        for (let i = 0; i < list.length; i++) {
-          expect(list[i]).to.not.equal(gitLabOrgApprovalList);
-        }
+        expect(list, 'gitlabOrgApprovalList').to.be.an('array');
+        expect(list).to.not.include(gitLabOrgApprovalList);

---

703-714: Domain add: prefer array include over loop + flag.

Reduces complexity and matches earlier tests.

-      let found = false;
-      for (let i = 0; i < list.length; i++) {
-        if (list[i] === domainApprovalList) {
-          expect(list[i]).to.eql(domainApprovalList);
-          found = true;
-          break;
-        }
-      }
-      if (!found) {
-        expect.fail('Domain not found in approval list');
-      }
+      expect(list, 'domainApprovalList').to.be.an('array');
+      expect(list).to.include(domainApprovalList);

---

734-736: Domain removal: collapse loop to a single assertion.

Cleaner and consistent with other removals.

-        for (let i = 0; i < list.length; i++) {
-          expect(list[i]).to.not.equal(domainApprovalList);
-        }
+        expect(list, 'domainApprovalList').to.be.an('array');
+        expect(list).to.not.include(domainApprovalList);

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 488243d and e6a73ce.

📒 Files selected for processing (1)

• tests/functional/cypress/e2e/v4/signatures.cy.ts (10 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: build-test-lint
• GitHub Check: cypress-functional