Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: implement new graphql fields for spec counts #25757

Merged
merged 31 commits into from
Feb 13, 2023
+290 −37
Merged

fix: implement new graphql fields for spec counts #25757

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
df76d1c
add new fixture
marktnoonan Feb 6, 2023
06ca95e
update comment
marktnoonan Feb 6, 2023
55abda9
add unit test based on current functionality
marktnoonan Feb 6, 2023
97d560a
add second test case based on current logic
marktnoonan Feb 6, 2023
cd88393
update test
marktnoonan Feb 6, 2023
5ae892f
[run ci] switch to new fields
marktnoonan Feb 6, 2023
89cbf00
run ci
marktnoonan Feb 6, 2023
75c030f
accept runNumber arg in relevantRunSpecChange
marktnoonan Feb 6, 2023
174e5f1
formatting
marktnoonan Feb 8, 2023
e502f21
revert runNumber arg
marktnoonan Feb 8, 2023
97131cf
commit updated graphql files
marktnoonan Feb 8, 2023
6ec7f87
update conditional logic and test
marktnoonan Feb 9, 2023
c13a6b3
[run ci] only invalidate cache when watching `current`
marktnoonan Feb 9, 2023
4b3a56b
Merge branch 'develop' into marktnoonan/25647-new-cloud-fields
marktnoonan Feb 9, 2023
945a66f
commit updated schema
marktnoonan Feb 9, 2023
0e72f2e
[run ci] - fix build time type error
marktnoonan Feb 10, 2023
9d520fb
Merge branch 'develop' into marktnoonan/25647-new-cloud-fields
marktnoonan Feb 10, 2023
0a727c3
fix types in stubs
marktnoonan Feb 10, 2023
0f2712e
add new properties to query
marktnoonan Feb 10, 2023
6f53db7
test cleanup
marktnoonan Feb 10, 2023
94a33c3
Merge branch 'develop' into marktnoonan/25647-new-cloud-fields
marktnoonan Feb 10, 2023
b4a1a0f
fix condition for cache clearing
marktnoonan Feb 10, 2023
e7a4059
Merge branch 'marktnoonan/25647-new-cloud-fields' of https://github.c…
marktnoonan Feb 10, 2023
83a95b9
update changelog
marktnoonan Feb 10, 2023
762dc2d
update changelog
marktnoonan Feb 10, 2023
97b2b43
Update packages/data-context/test/unit/sources/RelevantRunSpecsDataSo…
marktnoonan Feb 10, 2023
ae96ae7
Update cli/CHANGELOG.md
marktnoonan Feb 10, 2023
8c1e747
updates from feedback
marktnoonan Feb 10, 2023
f138cc3
make helper for repeated code
marktnoonan Feb 10, 2023
e6c08fa
Merge branch 'develop' into marktnoonan/25647-new-cloud-fields
marktnoonan Feb 13, 2023
46e79a5
remove bang
marktnoonan Feb 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cli/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ _Released 02/14/2023 (PENDING)_

- Fixed an issue with the Cloud project selection modal not showing the correct prompts. Fixes [#25520](https://github.com/cypress-io/cypress/issues/25520).
- Fixed an issue in middleware where error-handling code could itself generate an error and fail to report the original issue. Fixes [#22825](https://github.com/cypress-io/cypress/issues/22825).
- Fixed an issue in the Debug page where in-progress runs displayed a different count of specs locally compared to the count in Cypress Cloud. Fixes [#25647](https://github.com/cypress-io/cypress/issues/25647).
- Fixed an issue that could cause the Debug page to display a different number of specs for in-progress runs than shown in Cypress Cloud. Fixes [#25647](https://github.com/cypress-io/cypress/issues/25647).

**Features:**

Expand Down
43 changes: 17 additions & 26 deletions packages/data-context/src/sources/RelevantRunSpecsDataSource.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,35 +141,27 @@ export class RelevantRunSpecsDataSource {

const { current, next } = cloudProject

if (
current
&& current.runNumber
&& current.status
&& typeof current.totalInstanceCount === 'number'
&& typeof current.completedInstanceCount === 'number'
) {
runSpecsToReturn.runSpecs.current = {
totalSpecs: current.totalInstanceCount,
completedSpecs: current.completedInstanceCount,
runNumber: current.runNumber,
const formatCloudRunInfo = (cloudRunDetails: Partial<CloudRun>) => {
const { runNumber, totalInstanceCount, completedInstanceCount } = cloudRunDetails

if (runNumber && Number.isFinite(totalInstanceCount) && Number.isFinite(completedInstanceCount)) {
return {
totalSpecs: totalInstanceCount!,
completedSpecs: completedInstanceCount!,
runNumber,
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am still wrapping my head around the best way to do type checking in scenarios like this to confirm that variables are defined. It stinks that you had to use the ! to tell typescript that these variables have a value since the Number.isFinite does not do that for you.

This is just a comment and not a blocking request.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yuck, hadn't considered the typecheck loss. Could use _.isNumber which has an appropriate type guard, but that accepts NaN and *Infinity which isn't ideal. I think we'd have to build a custom type guard to handle our definition of a valid number to get the TS compiler to handle this automatically:

function isValidNumber(value: unknown): value is number {
  return Number.isFinite(value)
}
...
if (runNumber && isValidNumber(totalInstanceCount) && isValidNumber(completedInstanceCount)) {
  return {
    totalSpecs: totalInstanceCount,
    completedSpecs: completedInstanceCount,
    runNumber,
  }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that seems better than the !

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

}

runSpecsToReturn.statuses.current = current.status
return undefined
}

if (
next
&& next.runNumber
&& next.status
&& typeof next.totalInstanceCount === 'number'
&& typeof next.completedInstanceCount === 'number'
) {
runSpecsToReturn.runSpecs.next = {
totalSpecs: next.totalInstanceCount,
completedSpecs: next.completedInstanceCount,
runNumber: next.runNumber,
}
if (current && current.status) {
runSpecsToReturn.runSpecs.current = formatCloudRunInfo(current)
runSpecsToReturn.statuses.current = current.status
}

if (next && next.status) {
runSpecsToReturn.runSpecs.next = formatCloudRunInfo(next)
runSpecsToReturn.statuses.next = next.status
}

Expand All @@ -195,6 +187,7 @@ export class RelevantRunSpecsDataSource {

debug(`Spec data is `, specs)

const wasWatchingCurrentProject = this.#cached.statuses.current === 'RUNNING'
const specCountsChanged = !isEqual(specs.runSpecs, this.#cached.runSpecs)
const statusesChanged = !isEqual(specs.statuses, this.#cached.statuses)

Expand All @@ -210,8 +203,6 @@ export class RelevantRunSpecsDataSource {
debug('Run statuses changed')
const projectSlug = await this.ctx.project.projectId()

const wasWatchingCurrentProject = this.#cached.statuses.current === 'RUNNING'

if (projectSlug && wasWatchingCurrentProject) {
debug(`Invalidate cloudProjectBySlug ${projectSlug}`)
await this.ctx.cloud.invalidate('Query', 'cloudProjectBySlug', { slug: projectSlug })
Expand Down
2 changes: 1 addition & 1 deletion packages/data-context/test/unit/sources/RelevantRunSpecsDataSource.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { createTestDataContext } from '../helper'
import { RelevantRunSpecsDataSource, SPECS_EMPTY_RETURN } from '../../../src/sources'
import { FAKE_PROJECT_ONE_RUNNING_RUN_ONE_COMPLETED_THREE_SPECS, FAKE_PROJECT_ONE_RUNNING_RUN_ONE_SPEC } from './fixtures/graphqlFixtures'

describe('RelevantRunsDataSource', () => {
describe('RelevantRunSpecsDataSource', () => {
let ctx: DataContext
let dataSource: RelevantRunSpecsDataSource

Expand Down