test: address Cypress PR feedback

Author: dcoric

CYPRESS_PLAN.md

@@ -30,7 +30,6 @@ describe('Push Actions (Approve, Reject, Cancel)', () => {
   };
 
   before(() => {
-    // Setup: login as admin, create test users, assign permissions
     cy.login('admin', 'admin');
 
     cy.createUser(testUser.username, testUser.password, testUser.email, testUser.gitAccount);
@@ -50,15 +49,13 @@ describe('Push Actions (Approve, Reject, Cancel)', () => {
   });
 
   afterEach(function () {
-    // Clean up push created in this test (if any)
     if (this.pushId) {
       cy.deleteTestPush(this.pushId);
     }
     cy.logout();
   });
 
   after(() => {
-    // Clean up test users
     cy.deleteTestUser(testUser.username);
     cy.deleteTestUser(approverUser.username);
   });
@@ -73,43 +70,32 @@ describe('Push Actions (Approve, Reject, Cancel)', () => {
       cy.login(approverUser.username, approverUser.password);
       cy.visit(`/dashboard/push/${this.pushId}`);
 
-      // Verify push is Pending
       cy.get('[data-testid="push-status"]').should('contain', 'Pending');
 
-      // Action buttons should be visible
       cy.get('[data-testid="push-cancel-btn"]').should('be.visible');
       cy.get('[data-testid="push-reject-btn"]').should('be.visible');
       cy.get('[data-testid="attestation-open-btn"]').should('be.visible');
 
-      // Open attestation dialog
       cy.get('[data-testid="attestation-open-btn"]').click();
       cy.get('[data-testid="attestation-dialog"]').should('be.visible');
 
-      // Confirm button should be disabled until all checkboxes are checked
       cy.get('[data-testid="attestation-confirm-btn"]').should('be.disabled');
 
-      // Check all attestation checkboxes
       cy.get('[data-testid="attestation-dialog"]')
         .find('input[type="checkbox"]')
         .each(($checkbox) => {
           cy.wrap($checkbox).check({ force: true });
         });
 
-      // Confirm button should now be enabled
       cy.get('[data-testid="attestation-confirm-btn"]').should('not.be.disabled');
-
-      // Click confirm to approve
       cy.get('[data-testid="attestation-confirm-btn"]').click();
 
-      // Should navigate back to push list
       cy.url().should('include', '/dashboard/push');
       cy.url().should('not.include', this.pushId);
 
-      // Verify push is now Approved by revisiting its detail page
       cy.visit(`/dashboard/push/${this.pushId}`);
       cy.get('[data-testid="push-status"]').should('contain', 'Approved');
 
-      // Action buttons should no longer be visible for an approved push
       cy.get('[data-testid="push-cancel-btn"]').should('not.exist');
       cy.get('[data-testid="push-reject-btn"]').should('not.exist');
       cy.get('[data-testid="attestation-open-btn"]').should('not.exist');
@@ -126,33 +112,22 @@ describe('Push Actions (Approve, Reject, Cancel)', () => {
       cy.login(approverUser.username, approverUser.password);
       cy.visit(`/dashboard/push/${this.pushId}`);
 
-      // Verify push is Pending
       cy.get('[data-testid="push-status"]').should('contain', 'Pending');
 
-      // Open reject dialog
       cy.get('[data-testid="push-reject-btn"]').click();
 
-      // Confirm button should be disabled until reason is provided
       cy.get('[data-testid="push-reject-confirm-btn"]').should('be.disabled');
-
-      // Fill in rejection reason
       cy.get('#reason').type('Rejecting for test purposes');
 
-      // Confirm button should now be enabled
       cy.get('[data-testid="push-reject-confirm-btn"]').should('not.be.disabled');
-
-      // Confirm rejection
       cy.get('[data-testid="push-reject-confirm-btn"]').click();
 
-      // Should navigate back to push list
       cy.url().should('include', '/dashboard/push');
       cy.url().should('not.include', this.pushId);
 
-      // Verify push is now Rejected
       cy.visit(`/dashboard/push/${this.pushId}`);
       cy.get('[data-testid="push-status"]').should('contain', 'Rejected');
 
-      // Action buttons should no longer be visible
       cy.get('[data-testid="push-cancel-btn"]').should('not.exist');
       cy.get('[data-testid="push-reject-btn"]').should('not.exist');
       cy.get('[data-testid="attestation-open-btn"]').should('not.exist');
@@ -166,24 +141,17 @@ describe('Push Actions (Approve, Reject, Cancel)', () => {
     });
 
     it('should cancel a pending push', function () {
-      // Cancel can be done by the push author
       cy.login(testUser.username, testUser.password);
       cy.visit(`/dashboard/push/${this.pushId}`);
 
-      // Verify push is Pending
       cy.get('[data-testid="push-status"]').should('contain', 'Pending');
-
-      // Click Cancel
       cy.get('[data-testid="push-cancel-btn"]').click();
 
-      // Should navigate back to push list
       cy.url().should('include', '/dashboard/push');
 
-      // Verify push is now Canceled
       cy.visit(`/dashboard/push/${this.pushId}`);
       cy.get('[data-testid="push-status"]').should('contain', 'Canceled');
 
-      // Action buttons should no longer be visible
       cy.get('[data-testid="push-cancel-btn"]').should('not.exist');
       cy.get('[data-testid="push-reject-btn"]').should('not.exist');
       cy.get('[data-testid="attestation-open-btn"]').should('not.exist');
@@ -192,25 +160,19 @@ describe('Push Actions (Approve, Reject, Cancel)', () => {
 
   describe('Negative: unauthorized approve', () => {
     beforeEach(() => {
-      // Rate-limit guard: wait to avoid 429 from rapid push creations
-      // eslint-disable-next-line cypress/no-unnecessary-waiting
-      cy.wait(2000);
       const suffix = `neg-approve-${Date.now()}`;
       cy.createPush(testUser.username, testUser.password, testUser.email, suffix).as('pushId');
     });
 
     it('should not change push state when user lacks canAuthorise permission', function () {
-      // Login as testuser (has canPush but NOT canAuthorise)
       cy.login(testUser.username, testUser.password);
       cy.visit(`/dashboard/push/${this.pushId}`);
 
       cy.get('[data-testid="push-status"]').should('contain', 'Pending');
 
-      // Open attestation dialog and attempt to approve
       cy.get('[data-testid="attestation-open-btn"]').click();
       cy.get('[data-testid="attestation-dialog"]').should('be.visible');
 
-      // Check all checkboxes
       cy.get('[data-testid="attestation-dialog"]')
         .find('input[type="checkbox"]')
         .each(($checkbox) => {
@@ -219,46 +181,33 @@ describe('Push Actions (Approve, Reject, Cancel)', () => {
 
       cy.get('[data-testid="attestation-confirm-btn"]').click();
 
-      // TODO: The server correctly returns 403 but the UI (src/ui/services/git-push.ts)
-      // only handles 401 errors in authorisePush/rejectPush. The 403 is silently
-      // ignored and the user is navigated away without feedback. Once the UI properly
-      // handles 403, this test should assert a snackbar error message is shown.
+      // TODO: Assert snackbar feedback when the UI handles 403 push action responses.
       cy.visit(`/dashboard/push/${this.pushId}`);
       cy.get('[data-testid="push-status"]').should('contain', 'Pending');
     });
   });
 
   describe('Negative: unauthorized reject', () => {
     beforeEach(() => {
-      // Rate-limit guard: wait to avoid 429 from rapid push creations
-      // eslint-disable-next-line cypress/no-unnecessary-waiting
-      cy.wait(2000);
       const suffix = `neg-reject-${Date.now()}`;
       cy.createPush(testUser.username, testUser.password, testUser.email, suffix).as('pushId');
     });
 
     it('should not change push state when user lacks canAuthorise permission', function () {
-      // Login as testuser
       cy.login(testUser.username, testUser.password);
       cy.visit(`/dashboard/push/${this.pushId}`);
 
       cy.get('[data-testid="push-status"]').should('contain', 'Pending');
-
-      // Click Reject
       cy.get('[data-testid="push-reject-btn"]').click();
 
-      // TODO: Same issue as unauthorized approve — UI ignores 403 from server.
-      // Once fixed, assert snackbar error message is shown.
+      // TODO: Assert snackbar feedback when the UI handles 403 push action responses.
       cy.visit(`/dashboard/push/${this.pushId}`);
       cy.get('[data-testid="push-status"]').should('contain', 'Pending');
     });
   });
 
   describe('Attestation dialog cancel does not cancel the push', () => {
     beforeEach(() => {
-      // Rate-limit guard: wait to avoid 429 from rapid push creations
-      // eslint-disable-next-line cypress/no-unnecessary-waiting
-      cy.wait(2000);
       const suffix = `dialog-cancel-${Date.now()}`;
       cy.createPush(testUser.username, testUser.password, testUser.email, suffix).as('pushId');
     });
@@ -269,18 +218,14 @@ describe('Push Actions (Approve, Reject, Cancel)', () => {
 
       cy.get('[data-testid="push-status"]').should('contain', 'Pending');
 
-      // Open attestation dialog
       cy.get('[data-testid="attestation-open-btn"]').click();
       cy.get('[data-testid="attestation-dialog"]').should('be.visible');
 
-      // Click the dialog's Cancel button (NOT the push cancel button)
       cy.get('[data-testid="attestation-cancel-btn"]').click();
 
-      // Dialog should close, push should still be pending
       cy.get('[data-testid="attestation-dialog"]').should('not.exist');
       cy.get('[data-testid="push-status"]').should('contain', 'Pending');
 
-      // Action buttons should still be visible (push is still pending)
       cy.get('[data-testid="push-cancel-btn"]').should('be.visible');
       cy.get('[data-testid="push-reject-btn"]').should('be.visible');
       cy.get('[data-testid="attestation-open-btn"]').should('be.visible');

cypress/e2e/docker/pushActions.cy.js

@@ -26,18 +26,13 @@ describe('Error Pages', () => {
   afterEach(() => {
     cy.logout();
   });
-
-  // --- 9.1 Unknown route shows 404 ---
-  it('9.1 — Unknown route shows 404 page', () => {
+  it('Unknown route shows 404 page', () => {
     cy.visit('/dashboard/nonexistent-page-xyz');
 
     cy.get('[data-testid="not-found-page"]').should('be.visible');
     cy.contains('404').should('be.visible');
   });
-
-  // --- 9.2 Unauthorized route shows NotAuthorized ---
-  it('9.2 — Unauthorized route shows NotAuthorized page', () => {
-    // Create a non-admin user and try to access admin route
+  it('Unauthorized route shows NotAuthorized page', () => {
     const regularUser = {
       username: `errorpage_user_${Date.now()}`,
       password: 'pass123',
@@ -55,14 +50,10 @@ describe('Error Pages', () => {
     cy.clearCookies();
     cy.clearLocalStorage();
     cy.login(regularUser.username, regularUser.password);
-
-    // Navigate to not-authorized page directly to verify it renders
     cy.visit('/not-authorized');
 
     cy.get('[data-testid="not-authorized-page"]').should('be.visible');
     cy.contains('403').should('be.visible');
-
-    // Clean up user
     cy.clearCookies();
     cy.deleteTestUser(regularUser.username);
   });

cypress/e2e/error-pages.cy.js

@@ -26,72 +26,44 @@ describe('Navigation & Shell', () => {
   afterEach(() => {
     cy.logout();
   });
-
-  // --- 8.1 Sidebar renders all visible links ---
-  it('8.1 — Sidebar renders all visible links', () => {
+  it('Sidebar renders all visible links', () => {
     cy.visit('/dashboard/repo');
-
-    // Sidebar links should be present
     cy.contains('Repositories').should('be.visible');
     cy.contains('Dashboard').should('be.visible');
   });
-
-  // --- 8.2 Sidebar links navigate correctly ---
-  it('8.2 — Sidebar links navigate correctly', () => {
+  it('Sidebar links navigate correctly', () => {
     cy.visit('/dashboard/repo');
-
-    // Navigate to push dashboard
     cy.contains('Dashboard').click();
     cy.url().should('include', '/dashboard/push');
-
-    // Navigate back to repos
     cy.contains('Repositories').click();
     cy.url().should('include', '/dashboard/repo');
   });
-
-  // --- 8.3 Active sidebar item highlights ---
-  it('8.3 — Active sidebar item highlights', () => {
+  it('Active sidebar item highlights', () => {
     cy.visit('/dashboard/repo');
-
-    // The active nav link should have aria-current="page"
     cy.get('[aria-current="page"]').should('exist');
   });
-
-  // --- 8.4 Navbar renders ---
-  it('8.4 — Navbar renders correctly', () => {
+  it('Navbar renders correctly', () => {
     cy.visit('/dashboard/repo');
 
     cy.get('[data-testid="navbar"]').should('be.visible');
   });
-
-  // --- 8.5 Footer renders ---
-  it('8.5 — Footer renders', () => {
+  it('Footer renders', () => {
     cy.visit('/dashboard/repo');
 
     cy.get('[data-testid="footer"]').should('exist');
     cy.get('[data-testid="footer"]').scrollIntoView();
     cy.get('[data-testid="footer"]').should('be.visible');
   });
-
-  // --- 8.6 Unauthenticated user redirected ---
-  // NOTE: This test must run WITHOUT a login session, otherwise cy.session()
-  // caches authenticated cookies and restores them on cy.visit() despite
-  // cy.clearCookies() / cy.clearLocalStorage().  We place it in its own
-  // describe block that has no beforeEach login hook.
-
-  // --- 8.7 Root redirects to dashboard/repo ---
-  it('8.7 — / redirects to /dashboard/repo', () => {
+  // NOTE: Keep unauthenticated checks outside the logged-in hooks.
+  it('/ redirects to /dashboard/repo', () => {
     cy.logout();
     cy.visit('/');
-
-    // Root should redirect (either to login if not authenticated, or to dashboard/repo)
     cy.url().should('match', /\/(login|dashboard)/);
   });
 });
 
 describe('Unauthenticated access', () => {
-  it('8.6 — Unauthenticated user is redirected to /login', () => {
-    // Clear any saved Cypress sessions so no auth cookies are restored
+  it('Unauthenticated user is redirected to /login', () => {
     Cypress.session.clearAllSavedSessions();
     cy.clearCookies();
     cy.clearLocalStorage();