From f71ad89b02df80d9bf5e9c0da7e4a2c0ad8212dc Mon Sep 17 00:00:00 2001 From: Andrew Goldis Date: Wed, 5 Jul 2023 17:18:31 -0700 Subject: [PATCH] fix: prevent shared execution state --- .../webapp/cypress/e2e_smoke/smoke.spec.js | 3 + examples/webapp/scripts/currents-script.ts | 32 +++- .../cypress-cloud/lib/results/mapResult.ts | 5 +- packages/cypress-cloud/lib/results/results.ts | 21 +-- packages/cypress-cloud/lib/run.ts | 34 ++-- packages/cypress-cloud/lib/runner/index.ts | 1 - .../cypress-cloud/lib/runner/reportTask.ts | 34 ++-- packages/cypress-cloud/lib/runner/runner.ts | 56 ++++--- packages/cypress-cloud/lib/runner/state.ts | 145 ----------------- packages/cypress-cloud/lib/state/config.ts | 9 ++ packages/cypress-cloud/lib/state/execution.ts | 147 ++++++++++++++++++ packages/cypress-cloud/lib/state/index.ts | 2 + 12 files changed, 272 insertions(+), 217 deletions(-) delete mode 100644 packages/cypress-cloud/lib/runner/state.ts create mode 100644 packages/cypress-cloud/lib/state/config.ts create mode 100644 packages/cypress-cloud/lib/state/execution.ts create mode 100644 packages/cypress-cloud/lib/state/index.ts diff --git a/examples/webapp/cypress/e2e_smoke/smoke.spec.js b/examples/webapp/cypress/e2e_smoke/smoke.spec.js index b98c2023..21f66719 100644 --- a/examples/webapp/cypress/e2e_smoke/smoke.spec.js +++ b/examples/webapp/cypress/e2e_smoke/smoke.spec.js @@ -4,6 +4,9 @@ describe("TodoMVC", function () { // a very simple example helpful during presentations it("adds 2 todos", function () { + if (!!Cypress.env("CURRENTS_TESTING_FAIL")) { + throw new Error("oh!"); + } cy.get(".new-todo").type("learn testing{enter}").type("be cool{enter}"); cy.get(".todo-list li").should("have.length", 2); }); diff --git a/examples/webapp/scripts/currents-script.ts b/examples/webapp/scripts/currents-script.ts index 00455278..bb5ea047 100644 --- a/examples/webapp/scripts/currents-script.ts +++ b/examples/webapp/scripts/currents-script.ts @@ -5,17 +5,37 @@ import { run } from "cypress-cloud"; const projectId = process.env.CURRENTS_PROJECT_ID || "projectId"; const recordKey = process.env.CURRENTS_RECORD_KEY || "someKey"; - const result = await run({ - ciBuildId: `run-api-smoke-${new Date().toISOString()}`, + const ciBuildId = `run-api-smoke-${new Date().toISOString()}`; + const resultA = await run({ + ciBuildId, spec: ["cypress/e2e_smoke/**/*.spec.js"], projectId, recordKey, + group: "groupA", }); - if (result?.status === "failed") { + const resultB = await run({ + ciBuildId: ciBuildId + "b", + spec: ["cypress/e2e_smoke/**/*.spec.js"], + projectId, + recordKey, + group: "groupB", + env: { + CURRENTS_TESTING_FAIL: "yes", + }, + }); + + if (resultA?.status === "failed") { + process.exit(1); + } + assert(resultA?.runUrl?.match(/(\S+)/)); + assert(resultA?.totalPassed === 1); + assert(resultA?.totalTests === 1); + + if (resultB?.status === "failed") { process.exit(1); } - assert(result?.runUrl?.match(/(\S+)/)); - assert(result?.totalPassed === 1); - assert(result?.totalTests === 1); + assert(resultB?.totalPassed === 0); + assert(resultB?.totalFailed === 1); + assert(resultB?.totalTests === 1); })(); diff --git a/packages/cypress-cloud/lib/results/mapResult.ts b/packages/cypress-cloud/lib/results/mapResult.ts index a0e877bb..1b7aefce 100644 --- a/packages/cypress-cloud/lib/results/mapResult.ts +++ b/packages/cypress-cloud/lib/results/mapResult.ts @@ -6,7 +6,7 @@ import { } from "cypress-cloud/types"; import * as SpecAfter from "../runner/spec.type"; -import { getConfig } from "../runner/state"; +import { ConfigState } from "../state"; import { getFakeTestFromException } from "./results"; function getScreenshot(s: SpecAfter.Screenshot): CypressScreenshot { @@ -45,12 +45,13 @@ function getTest( } export function specResultsToCypressResults( + configState: ConfigState, specAfterResult: SpecAfter.SpecResult ): CypressCommandLine.CypressRunResult { return { status: "finished", // @ts-ignore - config: getConfig(), + config: configState.getConfig(), totalDuration: specAfterResult.stats.wallClockDuration, totalSuites: specAfterResult.stats.suites, totalTests: specAfterResult.stats.tests, diff --git a/packages/cypress-cloud/lib/results/results.ts b/packages/cypress-cloud/lib/results/results.ts index 6c59a6f3..1da7c0ef 100644 --- a/packages/cypress-cloud/lib/results/results.ts +++ b/packages/cypress-cloud/lib/results/results.ts @@ -8,7 +8,7 @@ import { UpdateInstanceResultsPayload, } from "../api"; import { MergedConfig } from "../config"; -import { getConfig } from "../runner/state"; +import { ConfigState } from "../state"; const debug = Debug("currents:results"); @@ -215,18 +215,21 @@ const getDummyFailedTest = (start: string, error: string) => ({ ], }); -export function getFailedDummyResult({ - specs, - error, -}: { - specs: string[]; - error: string; -}): CypressCommandLine.CypressRunResult { +export function getFailedDummyResult( + configState: ConfigState, + { + specs, + error, + }: { + specs: string[]; + error: string; + } +): CypressCommandLine.CypressRunResult { const start = new Date().toISOString(); const end = new Date().toISOString(); return { // @ts-ignore - config: getConfig() ?? {}, + config: configState.getConfig() ?? {}, status: "finished", startedTestsAt: new Date().toISOString(), endedTestsAt: new Date().toISOString(), diff --git a/packages/cypress-cloud/lib/run.ts b/packages/cypress-cloud/lib/run.ts index b739f67c..86d285da 100644 --- a/packages/cypress-cloud/lib/run.ts +++ b/packages/cypress-cloud/lib/run.ts @@ -21,21 +21,19 @@ import { pubsub } from "./pubsub"; import { summarizeTestResults, summaryTable } from "./results"; import { createReportTaskSpec, - getExecutionStateResults, reportTasks, runTillDoneOrCancelled, - setConfig, - setSpecAfter, - setSpecBefore, - setSpecOutput, } from "./runner"; import { shutdown } from "./shutdown"; import { getSpecFiles } from "./specMatcher"; +import { ConfigState, ExecutionState } from "./state"; import { startWSS } from "./ws"; const debug = Debug("currents:run"); export async function run(params: CurrentsRunParameters = {}) { + const executionState = new ExecutionState(); + const configState = new ConfigState(); activateDebug(params.cloudDebug); debug("run params %o", params); params = preprocessParams(params); @@ -62,9 +60,7 @@ export async function run(params: CurrentsRunParameters = {}) { } = validatedParams; const config = await getMergedConfig(validatedParams); - - // %state - setConfig(config?.resolved); + configState.setConfig(config?.resolved); const { specs, specPattern } = await getSpecFiles({ config, @@ -110,9 +106,11 @@ export async function run(params: CurrentsRunParameters = {}) { cutInitialOutput(); await startWSS(); - listenToSpecEvents(); + listenToSpecEvents(configState, executionState); await runTillDoneOrCancelled( + executionState, + configState, { runId: run.runId, groupId: run.groupId, @@ -126,7 +124,10 @@ export async function run(params: CurrentsRunParameters = {}) { divider(); await Promise.allSettled(reportTasks); - const _summary = summarizeTestResults(getExecutionStateResults(), config); + const _summary = summarizeTestResults( + executionState.getResults(configState), + config + ); title("white", "Cloud Run Finished"); console.log(summaryTable(_summary)); @@ -145,19 +146,22 @@ export async function run(params: CurrentsRunParameters = {}) { return _summary; } -function listenToSpecEvents() { +function listenToSpecEvents( + configState: ConfigState, + executionState: ExecutionState +) { pubsub.on("before:spec", async ({ spec }: { spec: Cypress.Spec }) => { debug("before:spec %o", spec); - setSpecBefore(spec.relative); + executionState.setSpecBefore(spec.relative); }); pubsub.on( "after:spec", async ({ spec, results }: { spec: Cypress.Spec; results: any }) => { debug("after:spec %o %o", spec, results); - setSpecAfter(spec.relative, results); - setSpecOutput(spec.relative, getCapturedOutput()); - createReportTaskSpec(spec.relative); + executionState.setSpecAfter(spec.relative, results); + executionState.setSpecOutput(spec.relative, getCapturedOutput()); + createReportTaskSpec(configState, executionState, spec.relative); } ); } diff --git a/packages/cypress-cloud/lib/runner/index.ts b/packages/cypress-cloud/lib/runner/index.ts index 71f2352f..f676566d 100644 --- a/packages/cypress-cloud/lib/runner/index.ts +++ b/packages/cypress-cloud/lib/runner/index.ts @@ -1,3 +1,2 @@ export * from "./cancellable"; export * from "./reportTask"; -export * from "./state"; diff --git a/packages/cypress-cloud/lib/runner/reportTask.ts b/packages/cypress-cloud/lib/runner/reportTask.ts index 2a818c62..cf4179b9 100644 --- a/packages/cypress-cloud/lib/runner/reportTask.ts +++ b/packages/cypress-cloud/lib/runner/reportTask.ts @@ -2,45 +2,49 @@ import { InstanceId } from "cypress-cloud/types"; import Debug from "debug"; import { error } from "../log"; import { getReportResultsTask } from "../results"; -import { - getExecutionStateInstance, - getExecutionStateSpec, - getInstanceResults, -} from "./state"; +import { ConfigState, ExecutionState } from "../state"; const debug = Debug("currents:reportTask"); export const reportTasks: Promise[] = []; -export const createReportTask = (instanceId: InstanceId) => { - const executionState = getExecutionStateInstance(instanceId); - if (!executionState) { +export const createReportTask = ( + configState: ConfigState, + executionState: ExecutionState, + instanceId: InstanceId +) => { + const instance = executionState.getInstance(instanceId); + if (!instance) { error("Cannot find execution state for instance %s", instanceId); return; } - if (executionState.reportStartedAt) { + if (instance.reportStartedAt) { debug("Report task already created for instance %s", instanceId); return; } - executionState.reportStartedAt = new Date(); + instance.reportStartedAt = new Date(); debug("Creating report task for instanceId %s", instanceId); reportTasks.push( getReportResultsTask( instanceId, - getInstanceResults(instanceId), - executionState.output ?? "no output captured" + executionState.getInstanceResults(configState, instanceId), + instance.output ?? "no output captured" ).catch(error) ); }; -export const createReportTaskSpec = (spec: string) => { - const i = getExecutionStateSpec(spec); +export const createReportTaskSpec = ( + configState: ConfigState, + executionState: ExecutionState, + spec: string +) => { + const i = executionState.getSpec(spec); if (!i) { error("Cannot find execution state for spec %s", spec); return; } debug("Creating report task for spec %s", spec); - return createReportTask(i.instanceId); + return createReportTask(configState, executionState, i.instanceId); }; diff --git a/packages/cypress-cloud/lib/runner/runner.ts b/packages/cypress-cloud/lib/runner/runner.ts index 46416376..30251074 100644 --- a/packages/cypress-cloud/lib/runner/runner.ts +++ b/packages/cypress-cloud/lib/runner/runner.ts @@ -17,16 +17,14 @@ import { import { runSpecFileSafe } from "../cypress"; import { isCurrents } from "../env"; import { divider, info, title, warn } from "../log"; +import { ConfigState, ExecutionState } from "../state"; import { createReportTask, reportTasks } from "./reportTask"; -import { - initExecutionState, - setInstanceOutput, - setInstanceResult, -} from "./state"; const debug = Debug("currents:runner"); export async function runTillDone( + executionState: ExecutionState, + configState: ConfigState, { runId, groupId, @@ -41,7 +39,7 @@ export async function runTillDone( let hasMore = true; while (hasMore) { - const newTasks = await runBatch({ + const newTasks = await runBatch(executionState, configState, { runMeta: { runId, groupId, @@ -56,24 +54,30 @@ export async function runTillDone( hasMore = false; break; } - newTasks.forEach((t) => createReportTask(t.instanceId)); + newTasks.forEach((t) => + createReportTask(configState, executionState, t.instanceId) + ); } } -async function runBatch({ - runMeta, - params, - allSpecs, -}: { - runMeta: { - runId: string; - groupId: string; - machineId: string; - platform: CreateInstancePayload["platform"]; - }; - allSpecs: SpecWithRelativeRoot[]; - params: ValidatedCurrentsParameters; -}) { +async function runBatch( + executionState: ExecutionState, + configState: ConfigState, + { + runMeta, + params, + allSpecs, + }: { + runMeta: { + runId: string; + groupId: string; + machineId: string; + platform: CreateInstancePayload["platform"]; + }; + allSpecs: SpecWithRelativeRoot[]; + params: ValidatedCurrentsParameters; + } +) { let batch = { specs: [] as InstanceResponseSpecDetails[], claimedInstances: 0, @@ -121,7 +125,7 @@ async function runBatch({ */ // %state - batch.specs.forEach(initExecutionState); + batch.specs.forEach((i) => executionState.initInstance(i)); divider(); info( @@ -148,12 +152,16 @@ async function runBatch({ // %state batch.specs.forEach((spec) => { - setInstanceOutput(spec.instanceId, output); + executionState.setInstanceOutput(spec.instanceId, output); const specRunResult = getCypressRunResultForSpec(spec.spec, rawResult); if (!specRunResult) { return; } - setInstanceResult(spec.instanceId, specRunResult); + executionState.setInstanceResult( + configState, + spec.instanceId, + specRunResult + ); }); resetCapture(); diff --git a/packages/cypress-cloud/lib/runner/state.ts b/packages/cypress-cloud/lib/runner/state.ts deleted file mode 100644 index 129f3bdd..00000000 --- a/packages/cypress-cloud/lib/runner/state.ts +++ /dev/null @@ -1,145 +0,0 @@ -import { InstanceId } from "cypress-cloud/types"; -import Debug from "debug"; -import { error, warn } from "../log"; -import { getFailedDummyResult } from "../results"; -import { - backfillException, - specResultsToCypressResults, -} from "../results/mapResult"; -import { SpecResult } from "./spec.type"; - -const debug = Debug("currents:state"); - -// Careful here - it is a global mutable state 🐲 -type InstanceExecutionState = { - instanceId: InstanceId; - spec: string; - output?: string; - specBefore?: Date; - createdAt: Date; - runResults?: CypressCommandLine.CypressRunResult; - runResultsReportedAt?: Date; - specAfter?: Date; - specAfterResults?: SpecResult; - reportStartedAt?: Date; -}; - -const executionState: Record = {}; - -export const getExecutionStateResults = () => - Object.values(executionState).map((i) => getInstanceResults(i.instanceId)); - -export const getExecutionStateSpec = (spec: string) => - Object.values(executionState).find((i) => i.spec === spec); - -export const getExecutionStateInstance = (instanceId: InstanceId) => - executionState[instanceId]; - -export const initExecutionState = ({ - spec, - instanceId, -}: { - instanceId: string; - spec: string; -}) => { - executionState[instanceId] = { - spec, - instanceId, - createdAt: new Date(), - }; -}; - -export const setSpecBefore = (spec: string) => { - const i = getExecutionStateSpec(spec); - if (!i) { - warn('Cannot find execution state for spec "%s"', spec); - return; - } - - i.specBefore = new Date(); -}; - -export const setSpecAfter = (spec: string, results: SpecResult) => { - const i = getExecutionStateSpec(spec); - if (!i) { - warn('Cannot find execution state for spec "%s"', spec); - return; - } - i.specAfter = new Date(); - i.specAfterResults = results; -}; - -export const setInstanceResult = ( - instanceId: string, - results: CypressCommandLine.CypressRunResult -) => { - const i = executionState[instanceId]; - if (!i) { - warn('Cannot find execution state for instance "%s"', instanceId); - return; - } - i.runResults = results; - i.runResultsReportedAt = new Date(); -}; - -export const setSpecOutput = (spec: string, output: string) => { - const i = getExecutionStateSpec(spec); - if (!i) { - warn('Cannot find execution state for spec "%s"', spec); - return; - } - setInstanceOutput(i.instanceId, output); -}; - -export const setInstanceOutput = (instanceId: string, output: string) => { - const i = executionState[instanceId]; - if (!i) { - warn('Cannot find execution state for instance "%s"', instanceId); - return; - } - if (i.output) { - debug('Instance "%s" already has output', instanceId); - return; - } - i.output = output; -}; - -export const hasResultsForSpec = (spec: string) => { - return Object.values(executionState).some( - (i) => i.spec === spec && i.runResults - ); -}; - -export const getInstanceResults = ( - instanceId: string -): CypressCommandLine.CypressRunResult => { - const i = getExecutionStateInstance(instanceId); - - if (!i) { - error('Cannot find execution state for instance "%s"', instanceId); - - return getFailedDummyResult({ - specs: ["unknown"], - error: "Cannot find execution state for instance", - }); - } - - // use spec:after results - it can become available before run results - if (i.specAfterResults) { - return backfillException(specResultsToCypressResults(i.specAfterResults)); - } - - if (i.runResults) { - return backfillException(i.runResults); - } - - debug('No results detected for "%s"', i.spec); - return getFailedDummyResult({ - specs: [i.spec], - error: `No results detected for the spec file. That usually happens because of cypress crash. See the console output for details.`, - }); -}; - -let _config: Cypress.ResolvedConfigOptions | undefined = undefined; -export const setConfig = (c: typeof _config) => (_config = c); -export const getConfig = () => _config; diff --git a/packages/cypress-cloud/lib/state/config.ts b/packages/cypress-cloud/lib/state/config.ts new file mode 100644 index 00000000..aa065652 --- /dev/null +++ b/packages/cypress-cloud/lib/state/config.ts @@ -0,0 +1,9 @@ +export class ConfigState { + private _config: Cypress.ResolvedConfigOptions | undefined = undefined; + public setConfig(c: typeof this._config) { + this._config = c; + } + public getConfig() { + return this._config; + } +} diff --git a/packages/cypress-cloud/lib/state/execution.ts b/packages/cypress-cloud/lib/state/execution.ts new file mode 100644 index 00000000..94cedbb4 --- /dev/null +++ b/packages/cypress-cloud/lib/state/execution.ts @@ -0,0 +1,147 @@ +import { InstanceId } from "cypress-cloud/types"; +import { error, warn } from "../log"; +import { getFailedDummyResult } from "../results"; +import { + backfillException, + specResultsToCypressResults, +} from "../results/mapResult"; +import { SpecResult } from "../runner/spec.type"; + +import Debug from "debug"; +import { ConfigState } from "./config"; +const debug = Debug("currents:state"); + +type InstanceExecutionState = { + instanceId: InstanceId; + spec: string; + output?: string; + specBefore?: Date; + createdAt: Date; + runResults?: CypressCommandLine.CypressRunResult; + runResultsReportedAt?: Date; + specAfter?: Date; + specAfterResults?: SpecResult; + reportStartedAt?: Date; +}; + +export class ExecutionState { + private state: Record = {}; + + public getResults(configState: ConfigState) { + return Object.values(this.state).map((i) => + this.getInstanceResults(configState, i.instanceId) + ); + } + + public getInstance(instanceId: InstanceId) { + return this.state[instanceId]; + } + + public getSpec(spec: string) { + return Object.values(this.state).find((i) => i.spec === spec); + } + + public initInstance({ + instanceId, + spec, + }: { + instanceId: InstanceId; + spec: string; + }) { + debug('Init execution state for "%s"', spec); + this.state[instanceId] = { + instanceId, + spec, + createdAt: new Date(), + }; + } + + public setSpecBefore(spec: string) { + const i = this.getSpec(spec); + if (!i) { + warn('Cannot find execution state for spec "%s"', spec); + return; + } + + i.specBefore = new Date(); + } + + public setSpecAfter(spec: string, results: SpecResult) { + const i = this.getSpec(spec); + if (!i) { + warn('Cannot find execution state for spec "%s"', spec); + return; + } + i.specAfter = new Date(); + i.specAfterResults = results; + } + + public setSpecOutput(spec: string, output: string) { + const i = this.getSpec(spec); + if (!i) { + warn('Cannot find execution state for spec "%s"', spec); + return; + } + this.setInstanceOutput(i.instanceId, output); + } + + public setInstanceOutput(instanceId: string, output: string) { + const i = this.state[instanceId]; + if (!i) { + warn('Cannot find execution state for instance "%s"', instanceId); + return; + } + if (i.output) { + debug('Instance "%s" already has output', instanceId); + return; + } + i.output = output; + } + + public setInstanceResult( + configState: ConfigState, + instanceId: string, + results: CypressCommandLine.CypressRunResult + ) { + const i = this.state[instanceId]; + if (!i) { + warn('Cannot find execution state for instance "%s"', instanceId); + return; + } + i.runResults = results; + i.runResultsReportedAt = new Date(); + } + + public getInstanceResults( + configState: ConfigState, + instanceId: string + ): CypressCommandLine.CypressRunResult { + const i = this.getInstance(instanceId); + + if (!i) { + error('Cannot find execution state for instance "%s"', instanceId); + + return getFailedDummyResult(configState, { + specs: ["unknown"], + error: "Cannot find execution state for instance", + }); + } + + // use spec:after results - it can become available before run results + if (i.specAfterResults) { + return backfillException( + specResultsToCypressResults(configState, i.specAfterResults) + ); + } + + if (i.runResults) { + return backfillException(i.runResults); + } + + debug('No results detected for "%s"', i.spec); + return getFailedDummyResult(configState, { + specs: [i.spec], + error: `No results detected for the spec file. That usually happens because of cypress crash. See the console output for details.`, + }); + } +} diff --git a/packages/cypress-cloud/lib/state/index.ts b/packages/cypress-cloud/lib/state/index.ts new file mode 100644 index 00000000..9786d2c0 --- /dev/null +++ b/packages/cypress-cloud/lib/state/index.ts @@ -0,0 +1,2 @@ +export * from "./config"; +export * from "./execution";