diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index cbc5a195a27c..9e48f9a81f30 100644 --- a/cli/CHANGELOG.md +++ b/cli/CHANGELOG.md @@ -6,6 +6,7 @@ _Released 07/05/2023 (PENDING)_ **Bugfixes:** - Fixed an issue where chrome was not recovering from browser crashes properly. Fixes [#24650](https://github.com/cypress-io/cypress/issues/24650). +- Fixed a race condition that was causing a GraphQL error to appear on the [Debug page](https://docs.cypress.io/guides/cloud/runs#Debug) when viewing a running Cypress Cloud build. Fixed in [#27134](https://github.com/cypress-io/cypress/pull/27134). ## 12.16.0 diff --git a/packages/app/src/debug/DebugTestingProgress.vue b/packages/app/src/debug/DebugTestingProgress.vue index 8b8b198e3d40..4de08d773aad 100644 --- a/packages/app/src/debug/DebugTestingProgress.vue +++ b/packages/app/src/debug/DebugTestingProgress.vue @@ -49,7 +49,7 @@ subscription DebugTestingProgress_Specs($id: ID!) { relevantRunSpecChange(runId: $id) { id ...DebugPendingRunCounts - scheduledToCompleteAt + scheduledToCompleteAt } } ` diff --git a/packages/data-context/src/polling/poller.ts b/packages/data-context/src/polling/poller.ts index c9ce9f0f7cba..ba78cbe9e43a 100644 --- a/packages/data-context/src/polling/poller.ts +++ b/packages/data-context/src/polling/poller.ts @@ -30,7 +30,7 @@ export class Poller { debug(`subscribing to ${this.event} with initial value %o and meta %o`, config?.initialValue, config?.meta) this.#subscriptions[subscriptionId] = { meta: config.meta } - debug('subscriptions before subscribe', this.#subscriptions) + debug('subscriptions after subscribe', this.#subscriptions) if (!this.#isPolling) { this.#isPolling = true @@ -57,13 +57,14 @@ export class Poller { } async #poll () { + debug(`polling for ${this.event}`) if (!this.#isPolling) { debug('terminating poll after being stopped') return } - debug(`polling for ${this.event} with interval of ${this.pollingInterval} seconds`) + debug(`calling poll callback for ${this.event}`) await this.callback(this.subscriptions) @@ -73,6 +74,7 @@ export class Poller { return } + debug(`setting timeout with interval of ${this.pollingInterval} second`) this.#timeout = setTimeout(async () => { this.#timeout = undefined await this.#poll() diff --git a/packages/data-context/src/sources/RelevantRunSpecsDataSource.ts b/packages/data-context/src/sources/RelevantRunSpecsDataSource.ts index 2f618a2efb12..9deffb20d65b 100644 --- a/packages/data-context/src/sources/RelevantRunSpecsDataSource.ts +++ b/packages/data-context/src/sources/RelevantRunSpecsDataSource.ts @@ -88,7 +88,8 @@ export class RelevantRunSpecsDataSource { //TODO Get spec counts before poll starts if (!this.#poller) { this.#poller = new Poller(this.ctx, 'relevantRunSpecChange', this.#pollingInterval, async (subscriptions) => { - debug('subscriptions', subscriptions) + debug('subscriptions', subscriptions.length) + const runIds = uniq(compact(subscriptions?.map((sub) => sub.meta?.runId))) debug('Polling for specs for runs: %o', runIds) @@ -128,13 +129,30 @@ export class RelevantRunSpecsDataSource { this.ctx.emitter.relevantRunSpecChange(run) } }) + + debug('processed runs') }) } + const fields = getFields(info) + + /** + * Checks runs as they are emitted for the subscription to make sure they match what is + * expected by the subscription they are attached to. First, verifies it is for the run + * being watched by comparing IDs. Then also verifies that the fields match. The field validation + * was needed to prevent a race condition when attaching different subscriptions for the same + * run that expect different fields. + * + * @param run check to see if it should be filtered out + */ const filter = (run: PartialCloudRunWithId) => { - debug('calling filter', run.id, runId) + const runIdsMatch = run.id === runId + const runFields = Object.keys(run) + const hasAllFields = fields.every((field) => runFields.includes(field)) + + debug('Calling filter %o', { runId, runIdsMatch, hasAllFields }, runFields, fields) - return run.id === runId + return runIdsMatch && hasAllFields } return this.#poller.start({ meta: { runId, info }, filter }) @@ -279,3 +297,15 @@ const createCombinedFragment = (name: string, fragmentNames: string[], type: Gra }, } } + +/** + * Get the field names for a given GraphQLResolveInfo + * @param info to extract field names from + */ +const getFields = (info: GraphQLResolveInfo) => { + const selections = info.fieldNodes[0]!.selectionSet?.selections! + + const fields = selections.map((selection) => selection.kind === 'Field' && selection.name.value).filter((field): field is string => typeof field === 'string') + + return fields +} diff --git a/packages/data-context/test/unit/sources/RelevantRunSpecsDataSource.spec.ts b/packages/data-context/test/unit/sources/RelevantRunSpecsDataSource.spec.ts index d561a40495bc..5fcc141fcd62 100644 --- a/packages/data-context/test/unit/sources/RelevantRunSpecsDataSource.spec.ts +++ b/packages/data-context/test/unit/sources/RelevantRunSpecsDataSource.spec.ts @@ -2,7 +2,7 @@ import chai from 'chai' import sinon from 'sinon' import sinonChai from 'sinon-chai' import debugLib from 'debug' -import { GraphQLString } from 'graphql' +import { GraphQLInt, GraphQLString } from 'graphql' import { DataContext } from '../../../src' import { createTestDataContext } from '../helper' @@ -47,7 +47,7 @@ describe('RelevantRunSpecsDataSource', () => { }) describe('polling', () => { - let clock + let clock: sinon.SinonFakeTimers beforeEach(() => { clock = sinon.useFakeTimers() @@ -57,45 +57,51 @@ describe('RelevantRunSpecsDataSource', () => { clock.restore() }) - const configureGraphQL = (cb: (info: Parameters[1]) => any) => { + it('polls and emits changes', async () => { + const testData = FAKE_PROJECT_ONE_RUNNING_RUN_ONE_SPEC + + //clone the fixture so it is not modified for future tests + const FAKE_PROJECT = JSON.parse(JSON.stringify(testData)) as typeof testData + + const runId = testData.data.cloudNodesByIds[0].id + + sinon.stub(ctx.cloud, 'executeRemoteGraphQL').resolves(FAKE_PROJECT) + const query = ` query Test { test { - value - value2 + id + runNumber + completedInstanceCount + totalInstanceCount + totalTests + status } } ` const fields = { - value: { + id: { type: GraphQLString, }, - value2: { + runNumber: { type: GraphQLString, }, - } - - return createGraphQL( - query, - fields, - (source, args, context, info) => { - return cb(info) + completedInstanceCount: { + type: GraphQLInt, }, - ) - } - - it('polls and emits changes', async () => { - const testData = FAKE_PROJECT_ONE_RUNNING_RUN_ONE_SPEC - - //clone the fixture so it is not modified for future tests - const FAKE_PROJECT = JSON.parse(JSON.stringify(testData)) as typeof testData - - const runId = testData.data.cloudNodesByIds[0].id - - sinon.stub(ctx.cloud, 'executeRemoteGraphQL').resolves(FAKE_PROJECT) + totalInstanceCount: { + type: GraphQLInt, + }, + totalTests: { + type: GraphQLInt, + }, + status: { + type: GraphQLString, + }, + } - return configureGraphQL(async (info) => { + return createGraphQL(query, fields, async (source, args, context, info) => { const subscriptionIterator = dataSource.pollForSpecs(runId, info) const firstEmit = await subscriptionIterator.next() @@ -145,8 +151,12 @@ describe('RelevantRunSpecsDataSource', () => { const expected = { data: { test: { - value: null, - value2: null, + completedInstanceCount: null, + id: null, + runNumber: null, + status: null, + totalInstanceCount: null, + totalTests: null, }, }, } @@ -155,14 +165,46 @@ describe('RelevantRunSpecsDataSource', () => { }) }) - it('should create query', () => { - const gqlStub = sinon.stub(ctx.cloud, 'executeRemoteGraphQL') + it('should create query', async () => { + const gqlStub = sinon.stub(ctx.cloud, 'executeRemoteGraphQL').resolves({ data: {} }) - return configureGraphQL(async (info) => { - const subscriptionIterator = dataSource.pollForSpecs('runId', info) + const fields = { + value: { + type: GraphQLString, + }, + value2: { + type: GraphQLString, + }, + value3: { + type: GraphQLString, + }, + } - subscriptionIterator.return(undefined) - }).then((result) => { + const query = ` + query Test { + test { + value + value2 + } + } + ` + + const query2 = ` + query Test { + test { + value2 + value3 + } + } + ` + + let iterator1: ReturnType + let iterator2: ReturnType + + return createGraphQL(query, fields, async (source, args, context, info) => { + iterator1 = dataSource.pollForSpecs('runId', info) + }) + .then(() => { const expected = dedent`query RelevantRunSpecsDataSource_Specs($ids: [ID!]!) { cloudNodesByIds(ids: $ids) { @@ -187,7 +229,51 @@ describe('RelevantRunSpecsDataSource', () => { expect(gqlStub).to.have.been.called expect(gqlStub.firstCall.args[0]).to.haveOwnProperty('operation') - expect(gqlStub.firstCall.args[0].operation).to.eql(`${expected }\n`) + expect(gqlStub.firstCall.args[0].operation, 'should match initial query with one fragment').to.eql(`${expected }\n`) + }) + .then(() => { + return createGraphQL(query2, fields, async (source, args, context, info) => { + iterator2 = dataSource.pollForSpecs('runId', info) + + await clock.nextAsync() + }) + }) + .then(() => { + const expected = + dedent`query RelevantRunSpecsDataSource_Specs($ids: [ID!]!) { + cloudNodesByIds(ids: $ids) { + id + ... on CloudRun { + ...Subscriptions + } + } + pollingIntervals { + runByNumber + } + } + + fragment Subscriptions on Test { + ...Fragment0 + ...Fragment1 + } + + fragment Fragment0 on Test { + value + value2 + } + + fragment Fragment1 on Test { + value2 + value3 + }` + + expect(gqlStub).to.have.been.calledTwice + expect(gqlStub.secondCall.args[0]).to.haveOwnProperty('operation') + expect(gqlStub.secondCall.args[0].operation, 'should match second query with two fragments').to.eql(`${expected }\n`) + }) + .then(() => { + iterator1.return(undefined) + iterator2.return(undefined) }) }) })