From 781a97c65f467f0a40042cd5e9804878a6487abe Mon Sep 17 00:00:00 2001 From: Dan Adajian Date: Mon, 25 Nov 2024 07:05:39 -0600 Subject: [PATCH] PR comments --- .gitignore | 3 +- app/backend/src/updateBaseImagesInS3.ts | 11 ++-- app/frontend/cypress/component/App.cy.tsx | 50 +++++++++++++------ app/frontend/cypress/downloads/downloads.html | 1 - 4 files changed, 46 insertions(+), 19 deletions(-) delete mode 100644 app/frontend/cypress/downloads/downloads.html diff --git a/.gitignore b/.gitignore index 6bcf4664..eed4f1ae 100644 --- a/.gitignore +++ b/.gitignore @@ -27,7 +27,8 @@ yarn-error.log* secrets.json -cypress/screenshots +app/frontend/cypress/downloads +app/frontend/cypress/screenshots app/frontend/cypress/videos .nx/ diff --git a/app/backend/src/updateBaseImagesInS3.ts b/app/backend/src/updateBaseImagesInS3.ts index 423f0840..fab5e151 100644 --- a/app/backend/src/updateBaseImagesInS3.ts +++ b/app/backend/src/updateBaseImagesInS3.ts @@ -28,10 +28,15 @@ export const updateBaseImagesInS3 = async ({ }); } const hash = commitHash ?? diffId; - if (hash) { - const s3Paths = await getKeysFromS3(hash, bucket); - await replaceImagesInS3(s3Paths, bucket); + if (!hash) { + throw new TRPCError({ + code: 'BAD_REQUEST', + message: 'Please provide either a commitHash or a diffId.' + }); } + + const s3Paths = await getKeysFromS3(hash, bucket); + await replaceImagesInS3(s3Paths, bucket); if (commitHash) { await updateCommitStatus({ owner, repo, commitHash }); } diff --git a/app/frontend/cypress/component/App.cy.tsx b/app/frontend/cypress/component/App.cy.tsx index 0b8473ee..4b743d41 100644 --- a/app/frontend/cypress/component/App.cy.tsx +++ b/app/frontend/cypress/component/App.cy.tsx @@ -54,7 +54,7 @@ describe('App', () => { it('should default to the diff image view of the first spec in the response list', () => { cy.findByRole('heading', { name: 'large/example' }); - cy.findByAltText('diff'); + cy.findByAltText('diff').should('be.visible'); cy.findByRole('button', { name: /back-arrow/ }).should('be.disabled'); }); @@ -64,30 +64,30 @@ describe('App', () => { }); it('should switch to different image views', () => { - cy.findByAltText('diff'); + cy.findByAltText('diff').should('be.visible'); cy.findByRole('button', { name: 'new' }).click(); - cy.findByAltText('new'); + cy.findByAltText('new').should('be.visible'); cy.findByRole('button', { name: 'base' }).click(); - cy.findByAltText('base'); + cy.findByAltText('base').should('be.visible'); }); it('should switch between specs and default to diff image for each one', () => { cy.findByRole('button', { name: /forward-arrow/ }).click(); cy.findByRole('heading', { name: 'small/example' }); - cy.findByAltText('diff'); + cy.findByAltText('diff').should('be.visible'); cy.findByRole('button', { name: /back-arrow/ }).click(); cy.findByRole('heading', { name: 'large/example' }); - cy.findByAltText('diff'); + cy.findByAltText('diff').should('be.visible'); }); it('should switch to side-by-side view and back', () => { cy.findByRole('button', { name: /forward-arrow/ }).click(); cy.findByRole('button', { name: /side-by-side/i }).should('be.enabled'); - cy.findByAltText('base'); - cy.findByAltText('diff'); - cy.findByAltText('new'); + cy.findByAltText('base').should('be.visible'); + cy.findByAltText('diff').should('be.visible'); + cy.findByAltText('new').should('be.visible'); cy.findByRole('button', { name: /single/i }).click(); - cy.findByAltText('diff'); + cy.findByAltText('diff').should('be.visible'); cy.findByAltText('base').should('not.exist'); cy.findByAltText('new').should('not.exist'); cy.findByRole('button', { name: /side-by-side/i }).should('be.enabled'); @@ -181,13 +181,13 @@ describe('App', () => { it('should display the new image with side-by-side view disabled', () => { cy.findByRole('heading', { name: 'large/new-example' }); cy.findByRole('button', { name: /new/ }); - cy.findByAltText('new'); + cy.findByAltText('new').should('be.visible'); cy.findByRole('button', { name: /side-by-side/i }).should('be.disabled'); }); it('should show new image with side-by-side view disabled when switching to another spec', () => { cy.findByRole('button', { name: /forward-arrow/ }).click(); - cy.findByAltText('new'); + cy.findByAltText('new').should('be.visible'); cy.findByRole('button', { name: /side-by-side/i }).should('be.disabled'); }); }); @@ -213,9 +213,31 @@ describe('App', () => { it('should default to diff when no new image was found and the currently selected image is new', () => { cy.findByRole('heading', { name: 'large/example' }); cy.findByRole('button', { name: /new/ }).click(); - cy.findByAltText('new'); + cy.findByAltText('new').should('be.visible'); cy.findByRole('button', { name: /forward-arrow/ }).click(); - cy.findByAltText('diff'); + cy.findByAltText('diff').should('be.visible'); + }); + }); + + describe('diffId param case', () => { + beforeEach(() => { + cy.intercept('/trpc/fetchCurrentPage*', req => { + const page = getPageFromRequest(req); + const body = page === 2 ? noNewImagesPage : firstPage; + req.reply(body); + }); + cy.mount( + + + + ); + }); + + it('should use diffId param when commitId not provided', () => { + cy.findByRole('heading', { name: 'large/example' }); + cy.findByAltText('diff').should('be.visible'); }); }); }); diff --git a/app/frontend/cypress/downloads/downloads.html b/app/frontend/cypress/downloads/downloads.html deleted file mode 100644 index 18d3040b..00000000 --- a/app/frontend/cypress/downloads/downloads.html +++ /dev/null @@ -1 +0,0 @@ -Cr24