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

misc: persist upload timings to cy cloud #28418

Merged
merged 14 commits into from
Nov 29, 2023
+137 −124
Merged

misc: persist upload timings to cy cloud #28418

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
ee407af
feat: report artifact upload durations to cy cloud for monitoring
cacieprins Nov 27, 2023
1ded84a
round upload durations to nearest ms
cacieprins Nov 28, 2023
6223f6a
update changelog
cacieprins Nov 28, 2023
6eeba62
Merge branch 'develop' into cacie/feat/persist-upload-timings
cacieprins Nov 28, 2023
65217ff
Merge branch 'develop' into cacie/feat/persist-upload-timings
cacieprins Nov 28, 2023
05ac027
revert changelog, fix typo in comment
cacieprins Nov 28, 2023
e758b6f
no longer round uploadDuration - api now accepts floats
cacieprins Nov 28, 2023
9badcb4
updates changelog
cacieprins Nov 28, 2023
a70b998
update api spec for refactored signature on api.updateInstanceArtifacts
cacieprins Nov 28, 2023
b9fefe0
Update CHANGELOG.md
cacieprins Nov 28, 2023
1125640
Update system-tests/test/record_spec.js
cacieprins Nov 28, 2023
36cb64c
rm defunct comment
cacieprins Nov 28, 2023
3102c39
Merge branch 'develop' into cacie/feat/persist-upload-timings
cacieprins Nov 28, 2023
2b65707
rm non-null accessors, this spec is not ts
cacieprins Nov 29, 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
6 changes: 5 additions & 1 deletion cli/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
<!-- See the ../guides/writing-the-cypress-changelog.md for details on writing the changelog. -->
## 13.6.1
## 13.7.0
chrisbreiding marked this conversation as resolved.
Show resolved Hide resolved

_Released 12/5/2023 (PENDING)_

**Features:**
Copy link
Member

Choose a reason for hiding this comment

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

This feels like a misc task that users don't care about. I wouldn't flag this as a feature since it's internal to us.

Copy link
Contributor Author

@cacieprins cacieprins Nov 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

updated in 9badcb4


- Artifact upload duration is now reported to Cypress Cloud. Addressed in [#28418](https://github.com/cypress-io/cypress/pull/28418)

**Bugfixes:**

- Fixed an issue where pages or downloads opened in a new tab were missing basic auth headers. Fixes [#28350](https://github.com/cypress-io/cypress/issues/28350).
Expand Down
47 changes: 23 additions & 24 deletions packages/server/lib/cloud/api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,28 +274,31 @@ type CreateRunResponse = {
} | undefined
}

type ArtifactMetadata = {
url: string
fileSize?: number
uploadDuration?: number
success: boolean
error?: string
}

type ProtocolMetadata = ArtifactMetadata & {
specAccess?: {
size: bigint
offset: bigint
}
}

type UpdateInstanceArtifactsPayload = {
screenshots: ArtifactMetadata[]
video?: ArtifactMetadata
protocol?: ProtocolMetadata
}

type UpdateInstanceArtifactsOptions = {
runId: string
instanceId: string
timeout: number | undefined
protocol: {
url: string
success: boolean
fileSize?: number | undefined
error?: string | undefined
} | undefined
screenshots: {
url: string
success: boolean
fileSize?: number | undefined
error?: string | undefined
}[] | undefined
video: {
url: string
success: boolean
fileSize?: number | undefined
error?: string | undefined
} | undefined
}

let preflightResult = {
Expand Down Expand Up @@ -497,17 +500,13 @@ module.exports = {
})
},

updateInstanceArtifacts (options: UpdateInstanceArtifactsOptions) {
updateInstanceArtifacts (options: UpdateInstanceArtifactsOptions, body: UpdateInstanceArtifactsPayload) {
return retryWithBackoff((attemptIndex) => {
return rp.put({
url: recordRoutes.instanceArtifacts(options.instanceId),
json: true,
timeout: options.timeout ?? SIXTY_SECONDS,
body: {
protocol: options.protocol,
screenshots: options.screenshots,
video: options.video,
},
body,
headers: {
'x-route-version': '1',
'x-cypress-run-id': options.runId,
Expand Down
18 changes: 10 additions & 8 deletions packages/server/lib/modes/record.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,10 @@ const uploadArtifactBatch = async (artifacts, protocolManager, quiet) => {
url: artifact.uploadUrl,
fileSize: artifact.fileSize,
key: artifact.reportKey,
duration: performance.now() - startTime,
// performance.now() gives milliseconds with many decimals - this is
// higher resolution than we need, and the cy cloud column is int, not
// a floating poina
uploadDuration: Math.round(performance.now() - startTime),
Copy link
Member

Choose a reason for hiding this comment

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

can we change this from duration to upload duration ? does this create an new column in the DB ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The new column has already been created in the DB, and the api schema has this field as uploadDuration

}
}

Expand All @@ -289,7 +292,7 @@ const uploadArtifactBatch = async (artifacts, protocolManager, quiet) => {
pathToFile: artifact.filePath,
fileSize: artifact.fileSize,
key: artifact.reportKey,
duration: performance.now() - startTime,
uploadDuration: Math.round(performance.now() - startTime),
}
} catch (err) {
debug('failed to upload artifact %o', {
Expand All @@ -308,7 +311,7 @@ const uploadArtifactBatch = async (artifacts, protocolManager, quiet) => {
allErrors: err.errors,
url: artifact.uploadUrl,
pathToFile: artifact.filePath,
duration: performance.now() - startTime,
uploadDuration: Math.round(performance.now() - startTime),
}
}

Expand All @@ -318,7 +321,7 @@ const uploadArtifactBatch = async (artifacts, protocolManager, quiet) => {
error: err.message,
url: artifact.uploadUrl,
pathToFile: artifact.filePath,
duration: performance.now() - startTime,
uploadDuration: Math.round(performance.now() - startTime),
}
}
}),
Expand Down Expand Up @@ -355,8 +358,7 @@ const uploadArtifactBatch = async (artifacts, protocolManager, quiet) => {
return skipped && !report.error ? acc : {
...acc,
[key]: {
// TODO: once cloud supports reporting duration, no longer omit this
..._.omit(report, 'duration'),
...report,
error,
},
}
Expand Down Expand Up @@ -452,8 +454,8 @@ const uploadArtifacts = async (options = {}) => {
try {
debug('upload reprt: %O', uploadReport)
const res = await api.updateInstanceArtifacts({
runId, instanceId, ...uploadReport,
})
runId, instanceId,
}, uploadReport)

return res
} catch (err) {
Expand Down
8 changes: 4 additions & 4 deletions packages/server/lib/util/print-run.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -614,11 +614,11 @@ type ArtifactUploadResultLike = {
success: boolean
error?: string
skipped?: boolean
duration?: number
uploadDuration?: number
}

export const printCompletedArtifactUpload = <T extends ArtifactUploadResultLike> (artifactUploadResult: T, labels: Record<'protocol' | 'screenshots' | 'video', string>, num: string): void => {
const { pathToFile, key, fileSize, success, error, skipped, duration } = artifactUploadResult
const { pathToFile, key, fileSize, success, error, skipped, uploadDuration } = artifactUploadResult

process.stdout.write(` - ${labels[key]} `)

Expand All @@ -630,8 +630,8 @@ export const printCompletedArtifactUpload = <T extends ArtifactUploadResultLike>
process.stdout.write(`- Failed Uploading`)
}

if (duration) {
const durationOut = humanTime.short(duration, 2)
if (uploadDuration) {
const durationOut = humanTime.short(uploadDuration, 2)

process.stdout.write(` ${success ? 'in' : 'after'} ${durationOut}`)
}
Expand Down
168 changes: 84 additions & 84 deletions system-tests/__snapshots__/record_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2844,90 +2844,6 @@ exports['e2e record capture-protocol disabled messaging displays disabled messag
Recorded Run: https://dashboard.cypress.io/projects/cjvoj7/runs/12
`

exports['e2e record capture-protocol enabled passing retrieves the capture protocol and uploads the db 1'] = `
====================================================================================================
(Run Starting)
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Cypress: 1.2.3 │
│ Browser: FooBrowser 88 │
│ Specs: 1 found (record_pass.cy.js) │
│ Searched: cypress/e2e/record_pass* │
│ Params: Tag: false, Group: false, Parallel: false │
│ Run URL: https://dashboard.cypress.io/projects/cjvoj7/runs/12 │
└────────────────────────────────────────────────────────────────────────────────────────────────┘
────────────────────────────────────────────────────────────────────────────────────────────────────
Running: record_pass.cy.js (1 of 1)
Estimated: X second(s)
record pass
✓ passes
- is pending
1 passing
1 pending
(Results)
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Tests: 2 │
│ Passing: 1 │
│ Failing: 0 │
│ Pending: 1 │
│ Skipped: 0 │
│ Screenshots: 1 │
│ Video: false │
│ Duration: X seconds │
│ Estimated: X second(s) │
│ Spec Ran: record_pass.cy.js │
└────────────────────────────────────────────────────────────────────────────────────────────────┘
(Screenshots)
- /XXX/XXX/XXX/cypress/screenshots/record_pass.cy.js/yay it passes.png (400x1022)
(Uploading Cloud Artifacts)
- Video - Nothing to upload
- Screenshot - 1 kB /XXX/XXX/XXX/cypress/screenshots/record_pass.cy.js/yay it passes.png
- Test Replay - 1 kB
Uploading Cloud Artifacts: . . . . .
(Uploaded Cloud Artifacts)
- Screenshot - Done Uploading 1 kB in Xm, Ys ZZ.ZZms 1/2 /XXX/XXX/XXX/cypress/screenshots/record_pass.cy.js/yay it passes.png
- Test Replay - Done Uploading 1 kB in Xm, Ys ZZ.ZZms 2/2
====================================================================================================
(Run Finished)
Spec Tests Passing Failing Pending Skipped
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ ✔ record_pass.cy.js XX:XX 2 1 - 1 - │
└────────────────────────────────────────────────────────────────────────────────────────────────┘
✔ All specs passed! XX:XX 2 1 - 1 -
───────────────────────────────────────────────────────────────────────────────────────────────────────
Recorded Run: https://dashboard.cypress.io/projects/cjvoj7/runs/12
`

exports['e2e record capture-protocol enabled when the tab crashes in chrome posts accurate test results 1'] = `
Expand Down Expand Up @@ -3904,3 +3820,87 @@ exports['capture-protocol api errors error report 500 continues 1'] = `
`

exports['e2e record capture-protocol enabled passing retrieves the capture protocol, uploads the db, and updates the artifact upload report 1'] = `
====================================================================================================
(Run Starting)
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Cypress: 1.2.3 │
│ Browser: FooBrowser 88 │
│ Specs: 1 found (record_pass.cy.js) │
│ Searched: cypress/e2e/record_pass* │
│ Params: Tag: false, Group: false, Parallel: false │
│ Run URL: https://dashboard.cypress.io/projects/cjvoj7/runs/12 │
└────────────────────────────────────────────────────────────────────────────────────────────────┘
────────────────────────────────────────────────────────────────────────────────────────────────────
Running: record_pass.cy.js (1 of 1)
Estimated: X second(s)
record pass
✓ passes
- is pending
1 passing
1 pending
(Results)
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Tests: 2 │
│ Passing: 1 │
│ Failing: 0 │
│ Pending: 1 │
│ Skipped: 0 │
│ Screenshots: 1 │
│ Video: false │
│ Duration: X seconds │
│ Estimated: X second(s) │
│ Spec Ran: record_pass.cy.js │
└────────────────────────────────────────────────────────────────────────────────────────────────┘
(Screenshots)
- /XXX/XXX/XXX/cypress/screenshots/record_pass.cy.js/yay it passes.png (400x1022)
(Uploading Cloud Artifacts)
- Video - Nothing to upload
- Screenshot - 1 kB /XXX/XXX/XXX/cypress/screenshots/record_pass.cy.js/yay it passes.png
- Test Replay - 1 kB
Uploading Cloud Artifacts: . . . . .
(Uploaded Cloud Artifacts)
- Screenshot - Done Uploading 1 kB in Xm, Ys ZZ.ZZms 1/2 /XXX/XXX/XXX/cypress/screenshots/record_pass.cy.js/yay it passes.png
- Test Replay - Done Uploading 1 kB in Xm, Ys ZZ.ZZms 2/2
====================================================================================================
(Run Finished)
Spec Tests Passing Failing Pending Skipped
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ ✔ record_pass.cy.js XX:XX 2 1 - 1 - │
└────────────────────────────────────────────────────────────────────────────────────────────────┘
✔ All specs passed! XX:XX 2 1 - 1 -
───────────────────────────────────────────────────────────────────────────────────────────────────────
Recorded Run: https://dashboard.cypress.io/projects/cjvoj7/runs/12
`
7 changes: 6 additions & 1 deletion system-tests/test/record_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2298,7 +2298,7 @@ describe('e2e record', () => {

describe('passing', () => {
enableCaptureProtocol()
it('retrieves the capture protocol and uploads the db', function () {
it('retrieves the capture protocol, uploads the db, and updates the artifact upload report', function () {
return systemTests.exec(this, {
key: 'f858a2bc-b469-4e48-be67-0876339ee7e1',
configFile: 'cypress-with-project-id.config.js',
Expand All @@ -2307,8 +2307,13 @@ describe('e2e record', () => {
snapshot: true,
}).then((ret) => {
const urls = getRequestUrls()
const artifactReport = getRequests().find(({ url }) => url === `PUT /instances/${instanceId}/artifacts`)?.body

expect(urls).to.include.members([`PUT ${CAPTURE_PROTOCOL_UPLOAD_URL}`])

expect(artifactReport?.protocol).to.exist()
expect(artifactReport?.protocol?.url).to.exist().and.not.be.empty()
expect(artifactReport?.protocol?.uploadDuration).to.exist()
cacieprins marked this conversation as resolved.
Show resolved Hide resolved
})
})
})
Expand Down
Loading