From 26edc21ce7b9858f82144068b74f81e67983146f Mon Sep 17 00:00:00 2001 From: Zach Panzarino Date: Fri, 19 Feb 2021 19:14:18 -0500 Subject: [PATCH] fix: studio save to definitions with config and surface save errs (#15069) --- packages/runner/src/lib/event-manager.js | 6 +- packages/runner/src/studio/studio-recorder.js | 19 +-- .../__snapshots__/spec_writer_spec.ts.js | 132 ++++++++++++++++++ packages/server/lib/socket-e2e.ts | 12 +- packages/server/lib/studio.ts | 26 +++- packages/server/lib/util/spec_writer.ts | 4 +- .../cypress/integration/unwritten.spec.js | 8 ++ .../server/test/unit/util/spec_writer_spec.ts | 20 ++- 8 files changed, 206 insertions(+), 21 deletions(-) diff --git a/packages/runner/src/lib/event-manager.js b/packages/runner/src/lib/event-manager.js index 154f81fc6dd1..5c4c4e67c38d 100644 --- a/packages/runner/src/lib/event-manager.js +++ b/packages/runner/src/lib/event-manager.js @@ -243,9 +243,9 @@ const eventManager = { }) localBus.on('studio:save', (saveInfo) => { - ws.emit('studio:save', saveInfo, (success) => { - if (!success) { - reporterBus.emit('test:set:state', studioRecorder.saveError, _.noop) + ws.emit('studio:save', saveInfo, (err) => { + if (err) { + reporterBus.emit('test:set:state', studioRecorder.saveError(err), _.noop) } }) }) diff --git a/packages/runner/src/studio/studio-recorder.js b/packages/runner/src/studio/studio-recorder.js index bef144f3d784..a3b2a34bf199 100644 --- a/packages/runner/src/studio/studio-recorder.js +++ b/packages/runner/src/studio/studio-recorder.js @@ -4,11 +4,14 @@ import $driverUtils from '@packages/driver/src/cypress/utils' import eventManager from '../lib/event-manager' -const saveErrorMessage = `\ +const saveErrorMessage = (message) => { + return `\ +${message}\n\n\ Cypress was unable to save these commands to your spec file. \ Cypress Studio is still in beta and the team is working hard to \ resolve issues like this. To help us fix this issue more quickly, \ you can provide us with more information by clicking 'Learn more' below.` +} const eventTypes = [ 'click', @@ -75,21 +78,21 @@ export class StudioRecorder { } } - @computed get saveError () { + get Cypress () { + return eventManager.getCypress() + } + + saveError (err) { return { id: this.testId, err: { - name: 'CypressError', - message: saveErrorMessage, + ...err, + message: saveErrorMessage(err.message), docsUrl: 'https://on.cypress.io/studio-beta', }, } } - get Cypress () { - return eventManager.getCypress() - } - @action setTestId = (testId) => { this.testId = testId } diff --git a/packages/server/__snapshots__/spec_writer_spec.ts.js b/packages/server/__snapshots__/spec_writer_spec.ts.js index 829ce0ad9faf..6d55ddf1de52 100644 --- a/packages/server/__snapshots__/spec_writer_spec.ts.js +++ b/packages/server/__snapshots__/spec_writer_spec.ts.js @@ -50,6 +50,10 @@ describe('top level suite', () => { it.only('test with it.only', () => { cy.get('.btn').click() }) + + it('test with config', { responseTimeout: 60000 }, () => { + cy.get('.btn').click() + }) }) context('inner suite with context', () => { @@ -60,6 +64,10 @@ describe('top level suite', () => { describe.only('inner suite with describe.only', () => { }) + + describe('suite with config', { responseTimeout: 60000 }, () => { + + }) }) ` @@ -83,6 +91,10 @@ describe('top level suite', () => { it.only('test with it.only', () => { cy.get('.btn').click() }) + + it('test with config', { responseTimeout: 60000 }, () => { + cy.get('.btn').click() + }) }) context('inner suite with context', () => { @@ -93,6 +105,10 @@ describe('top level suite', () => { describe.only('inner suite with describe.only', () => { }) + + describe('suite with config', { responseTimeout: 60000 }, () => { + + }) }) ` @@ -113,6 +129,10 @@ describe('top level suite', () => { cy.get('.btn').click() }) + it('test with config', { responseTimeout: 60000 }, () => { + cy.get('.btn').click() + }) + /* === Test Created with Cypress Studio === */ it('test added to describe', function() { /* ==== Generated with Cypress Studio ==== */ @@ -130,6 +150,10 @@ describe('top level suite', () => { describe.only('inner suite with describe.only', () => { }) + + describe('suite with config', { responseTimeout: 60000 }, () => { + + }) }) ` @@ -149,6 +173,10 @@ describe('top level suite', () => { it.only('test with it.only', () => { cy.get('.btn').click() }) + + it('test with config', { responseTimeout: 60000 }, () => { + cy.get('.btn').click() + }) }) context('inner suite with context', () => { @@ -165,6 +193,10 @@ describe('top level suite', () => { describe.only('inner suite with describe.only', () => { }) + + describe('suite with config', { responseTimeout: 60000 }, () => { + + }) }) ` @@ -188,6 +220,10 @@ describe('top level suite', () => { cy.get('.btn').click(); /* ==== End Cypress Studio ==== */ }) + + it('test with config', { responseTimeout: 60000 }, () => { + cy.get('.btn').click() + }) }) context('inner suite with context', () => { @@ -198,6 +234,10 @@ describe('top level suite', () => { describe.only('inner suite with describe.only', () => { }) + + describe('suite with config', { responseTimeout: 60000 }, () => { + + }) }) ` @@ -217,6 +257,10 @@ describe('top level suite', () => { it.only('test with it.only', () => { cy.get('.btn').click() }) + + it('test with config', { responseTimeout: 60000 }, () => { + cy.get('.btn').click() + }) }) context('inner suite with context', () => { @@ -233,6 +277,94 @@ describe('top level suite', () => { /* ==== End Cypress Studio ==== */ }); }) + + describe('suite with config', { responseTimeout: 60000 }, () => { + + }) +}) + +` + +exports['lib/util/spec_writer #appendCommandsToTest can add commands to an existing test with config 1'] = ` +describe('top level suite', () => { + describe('inner suite with describe', () => { + it('test with it', () => { + cy.get('.btn').click() + }) + + specify('test with specify', () => { + cy.get('.btn').click() + }) + + // eslint-disable-next-line mocha/no-exclusive-tests + it.only('test with it.only', () => { + cy.get('.btn').click() + }) + + it('test with config', { responseTimeout: 60000 }, () => { + cy.get('.btn').click() + /* ==== Generated with Cypress Studio ==== */ + cy.get('.input').type('typed text'); + cy.get('.btn').click(); + /* ==== End Cypress Studio ==== */ + }) + }) + + context('inner suite with context', () => { + + }) + + // eslint-disable-next-line mocha/no-exclusive-tests + describe.only('inner suite with describe.only', () => { + + }) + + describe('suite with config', { responseTimeout: 60000 }, () => { + + }) +}) + +` + +exports['lib/util/spec_writer #createNewTestInSuite can create a new test in a suite with config 1'] = ` +describe('top level suite', () => { + describe('inner suite with describe', () => { + it('test with it', () => { + cy.get('.btn').click() + }) + + specify('test with specify', () => { + cy.get('.btn').click() + }) + + // eslint-disable-next-line mocha/no-exclusive-tests + it.only('test with it.only', () => { + cy.get('.btn').click() + }) + + it('test with config', { responseTimeout: 60000 }, () => { + cy.get('.btn').click() + }) + }) + + context('inner suite with context', () => { + + }) + + // eslint-disable-next-line mocha/no-exclusive-tests + describe.only('inner suite with describe.only', () => { + + }) + + describe('suite with config', { responseTimeout: 60000 }, () => { + /* === Test Created with Cypress Studio === */ + it('test added to describe with config', function() { + /* ==== Generated with Cypress Studio ==== */ + cy.get('.input').type('typed text'); + cy.get('.btn').click(); + /* ==== End Cypress Studio ==== */ + }); + }) }) ` diff --git a/packages/server/lib/socket-e2e.ts b/packages/server/lib/socket-e2e.ts index ed118774c926..44d1253a109e 100644 --- a/packages/server/lib/socket-e2e.ts +++ b/packages/server/lib/socket-e2e.ts @@ -114,17 +114,19 @@ export class SocketE2E extends SocketBase { }) socket.on('studio:save', (saveInfo, cb) => { - // even if the user has turned off file watching - // we want to force a reload on save + // even if the user has turned off file watching + // we want to force a reload on save if (!config.watchForFileChanges) { preprocessor.emitter.on('file:updated', this.onStudioTestFileChange) } studio.save(saveInfo) - .then((success) => { - cb(success) + .then((err) => { + cb(err) - if (!success && !config.watchForFileChanges) { + // onStudioTestFileChange will remove itself after being called + // but if there's an error, it never gets called so we manually remove it + if (err && !config.watchForFileChanges) { this.removeOnStudioTestFileChange() } }) diff --git a/packages/server/lib/studio.ts b/packages/server/lib/studio.ts index 6ca975f521a2..9afffebbc312 100644 --- a/packages/server/lib/studio.ts +++ b/packages/server/lib/studio.ts @@ -8,6 +8,15 @@ interface SaveInfo { testName?: string } +class StudioSaveError extends Error { + static errMessage = (isSuite) => `Studio was unable to find your ${isSuite ? 'suite' : 'test'} in the spec file.` + + constructor (isSuite) { + super(StudioSaveError.errMessage(isSuite)) + this.name = 'StudioSaveError' + } +} + export const setStudioModalShown = () => { return savedState.create() .then((state) => { @@ -35,7 +44,20 @@ export const save = (saveInfo: SaveInfo) => { return saveToFile() .then((success) => { return setStudioModalShown() - .then(() => success) + .then(() => { + if (!success) { + throw new StudioSaveError(isSuite) + } + + return null + }) + }) + .catch((err) => { + return { + type: err.type, + name: err.name, + message: err.message, + stack: err.stack, + } }) - .catch(() => false) } diff --git a/packages/server/lib/util/spec_writer.ts b/packages/server/lib/util/spec_writer.ts index c952a26601b9..4519ddda318e 100644 --- a/packages/server/lib/util/spec_writer.ts +++ b/packages/server/lib/util/spec_writer.ts @@ -130,7 +130,9 @@ export const generateAstRules = (fileDetails: { line: number, column: number }, const columnEnd = identifier.loc.end.column + 2 if (fnNames.includes(identifier.name) && identifier.loc.start.line === line && columnStart <= column && column <= columnEnd) { - const fn = node.arguments[1] as n.FunctionExpression + const arg1 = node.arguments[1] + + const fn = (arg1.type === 'ObjectExpression' ? node.arguments[2] : arg1) as n.FunctionExpression if (!fn) { return false diff --git a/packages/server/test/support/fixtures/projects/studio/cypress/integration/unwritten.spec.js b/packages/server/test/support/fixtures/projects/studio/cypress/integration/unwritten.spec.js index f06b335b100c..37e0487df73d 100644 --- a/packages/server/test/support/fixtures/projects/studio/cypress/integration/unwritten.spec.js +++ b/packages/server/test/support/fixtures/projects/studio/cypress/integration/unwritten.spec.js @@ -12,6 +12,10 @@ describe('top level suite', () => { it.only('test with it.only', () => { cy.get('.btn').click() }) + + it('test with config', { responseTimeout: 60000 }, () => { + cy.get('.btn').click() + }) }) context('inner suite with context', () => { @@ -22,4 +26,8 @@ describe('top level suite', () => { describe.only('inner suite with describe.only', () => { }) + + describe('suite with config', { responseTimeout: 60000 }, () => { + + }) }) diff --git a/packages/server/test/unit/util/spec_writer_spec.ts b/packages/server/test/unit/util/spec_writer_spec.ts index 6c455e1c9f25..7d5432c4ead5 100644 --- a/packages/server/test/unit/util/spec_writer_spec.ts +++ b/packages/server/test/unit/util/spec_writer_spec.ts @@ -128,6 +128,14 @@ describe('lib/util/spec_writer', () => { column: 8, }, exampleTestCommands) }) + + it('can add commands to an existing test with config', () => { + appendCommandsToTest({ + absoluteFile: '', + line: 16, + column: 8, + }, exampleTestCommands) + }) }) describe('#createNewTestInSuite', () => { @@ -142,7 +150,7 @@ describe('lib/util/spec_writer', () => { it('can create a new test in a suite defined with context', () => { createNewTestInSuite({ absoluteFile: '', - line: 17, + line: 21, column: 3, }, exampleTestCommands, 'test added to context') }) @@ -150,9 +158,17 @@ describe('lib/util/spec_writer', () => { it('can create a new test in a suite defined with describe only', () => { createNewTestInSuite({ absoluteFile: '', - line: 22, + line: 26, column: 12, }, exampleTestCommands, 'test added to describe only') }) + + it('can create a new test in a suite with config', () => { + createNewTestInSuite({ + absoluteFile: '', + line: 30, + column: 12, + }, exampleTestCommands, 'test added to describe with config') + }) }) })