-
Notifications
You must be signed in to change notification settings - Fork 3.3k
perf: updates for potential reporter rendering performance #32002
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.
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
base: develop
Are you sure you want to change the base?
Changes from 6 commits
840ae62
2beb717
7b2af7a
f9a8df7
31f4bc3
72bffd2
1118376
e1c4cda
be5454a
5be09dd
75ab32e
5b7c30f
232b86d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,11 +20,7 @@ export function getReporterElement () { | |
} | ||
|
||
export function empty (el: HTMLElement) { | ||
while (el.lastChild) { | ||
if (el && el.firstChild) { | ||
el.removeChild(el.firstChild) | ||
} | ||
} | ||
el.innerHTML = '' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function is called to empty the |
||
} | ||
|
||
export const togglePlayground = (autIframe: AutIframe) => { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
<template> | ||
<div | ||
class="space-y-[32px] h-[calc(100vh-[64px])] p-[32px] overflow-auto" | ||
class="space-y-[32px] h-[calc(100vh-64px)] p-[32px] overflow-auto" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This tailwind class is invalid, just noticed it while doing other things. |
||
data-cy="settings" | ||
> | ||
<div class="space-y-[24px]"> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -409,11 +409,23 @@ class $Cypress { | |
this.emit('cypress:in:cypress:runner:event', ...args) | ||
} | ||
|
||
private _handleMochaEvent (eventType: string, args: unknown[], isTextTerminal: boolean) { | ||
this.maybeEmitCypressInCypress('mocha', eventType, ...args) | ||
|
||
if (isTextTerminal) { | ||
return this.emit('mocha', eventType, ...args) | ||
} | ||
} | ||
Comment on lines
+412
to
+418
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. refactor pulling this out |
||
|
||
action (eventName, ...args) { | ||
// normalizes all the various ways | ||
// other objects communicate intent | ||
// and 'action' to Cypress | ||
debug(eventName) | ||
|
||
const isTextTerminal = this.config('isTextTerminal') | ||
const isInteractive = this.config('isInteractive') | ||
Comment on lines
+426
to
+427
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pulling these out to just call once. |
||
|
||
switch (eventName) { | ||
case 'recorder:frame': | ||
return this.emit('recorder:frame', args[0]) | ||
|
@@ -434,13 +446,7 @@ class $Cypress { | |
return | ||
} | ||
|
||
this.maybeEmitCypressInCypress('mocha', 'start', args[0]) | ||
|
||
if (this.config('isTextTerminal')) { | ||
return this.emit('mocha', 'start', args[0]) | ||
} | ||
|
||
break | ||
return this._handleMochaEvent('start', args, isTextTerminal) | ||
|
||
case 'runner:end': | ||
$sourceMapUtils.destroySourceMapConsumers() | ||
|
@@ -451,110 +457,51 @@ class $Cypress { | |
// TODO: it would be nice to await this emit before preceding. | ||
this.emit('run:end') | ||
|
||
this.maybeEmitCypressInCypress('mocha', 'end', args[0]) | ||
|
||
if (this.config('isTextTerminal')) { | ||
return this.emit('mocha', 'end', args[0]) | ||
} | ||
|
||
break | ||
return this._handleMochaEvent('end', args, isTextTerminal) | ||
|
||
case 'runner:suite:start': | ||
// mocha runner started processing a suite | ||
this.maybeEmitCypressInCypress('mocha', 'suite', ...args) | ||
|
||
if (this.config('isTextTerminal')) { | ||
return this.emit('mocha', 'suite', ...args) | ||
} | ||
return this._handleMochaEvent('suite', args, isTextTerminal) | ||
|
||
break | ||
case 'runner:suite:end': | ||
// mocha runner finished processing a suite | ||
this.maybeEmitCypressInCypress('mocha', 'suite end', ...args) | ||
|
||
if (this.config('isTextTerminal')) { | ||
return this.emit('mocha', 'suite end', ...args) | ||
} | ||
return this._handleMochaEvent('suite end', args, isTextTerminal) | ||
|
||
break | ||
case 'runner:hook:start': | ||
// mocha runner started processing a hook | ||
|
||
this.maybeEmitCypressInCypress('mocha', 'hook', ...args) | ||
|
||
if (this.config('isTextTerminal')) { | ||
return this.emit('mocha', 'hook', ...args) | ||
} | ||
|
||
break | ||
return this._handleMochaEvent('hook', args, isTextTerminal) | ||
|
||
case 'runner:hook:end': | ||
// mocha runner finished processing a hook | ||
this.maybeEmitCypressInCypress('mocha', 'hook end', ...args) | ||
|
||
if (this.config('isTextTerminal')) { | ||
return this.emit('mocha', 'hook end', ...args) | ||
} | ||
|
||
break | ||
return this._handleMochaEvent('hook end', args, isTextTerminal) | ||
|
||
case 'runner:test:start': | ||
// mocha runner started processing a hook | ||
this.maybeEmitCypressInCypress('mocha', 'test', ...args) | ||
|
||
if (this.config('isTextTerminal')) { | ||
return this.emit('mocha', 'test', ...args) | ||
} | ||
|
||
break | ||
return this._handleMochaEvent('test', args, isTextTerminal) | ||
|
||
case 'runner:test:end': | ||
this.maybeEmitCypressInCypress('mocha', 'test end', ...args) | ||
|
||
if (this.config('isTextTerminal')) { | ||
return this.emit('mocha', 'test end', ...args) | ||
} | ||
|
||
break | ||
return this._handleMochaEvent('test end', args, isTextTerminal) | ||
|
||
case 'runner:pass': | ||
// mocha runner calculated a pass | ||
// this is delayed from when mocha would normally fire it | ||
// since we fire it after all afterEach hooks have ran | ||
this.maybeEmitCypressInCypress('mocha', 'pass', ...args) | ||
|
||
if (this.config('isTextTerminal')) { | ||
return this.emit('mocha', 'pass', ...args) | ||
} | ||
|
||
break | ||
return this._handleMochaEvent('pass', args, isTextTerminal) | ||
|
||
case 'runner:pending': | ||
// mocha runner calculated a pending test | ||
this.maybeEmitCypressInCypress('mocha', 'pending', ...args) | ||
|
||
if (this.config('isTextTerminal')) { | ||
return this.emit('mocha', 'pending', ...args) | ||
} | ||
|
||
break | ||
return this._handleMochaEvent('pending', args, isTextTerminal) | ||
|
||
case 'runner:fail': { | ||
this.maybeEmitCypressInCypress('mocha', 'fail', ...args) | ||
|
||
if (this.config('isTextTerminal')) { | ||
return this.emit('mocha', 'fail', ...args) | ||
} | ||
|
||
break | ||
return this._handleMochaEvent('fail', args, isTextTerminal) | ||
} | ||
// retry event only fired in mocha version 6+ | ||
// https://github.com/mochajs/mocha/commit/2a76dd7589e4a1ed14dd2a33ab89f182e4c4a050 | ||
case 'runner:retry': { | ||
// mocha runner calculated a pass | ||
this.maybeEmitCypressInCypress('mocha', 'retry', ...args) | ||
|
||
if (this.config('isTextTerminal')) { | ||
if (isTextTerminal) { | ||
this.emit('mocha', 'retry', ...args) | ||
} | ||
|
||
|
@@ -567,7 +514,7 @@ class $Cypress { | |
case 'runner:test:before:run': | ||
this.maybeEmitCypressInCypress('mocha', 'test:before:run', args[0]) | ||
|
||
if (this.config('isTextTerminal')) { | ||
if (isTextTerminal) { | ||
// needed for handling test retries | ||
this.emit('mocha', 'test:before:run', args[0]) | ||
} | ||
|
@@ -602,7 +549,7 @@ class $Cypress { | |
this.emit('test:after:run', ...args) | ||
this.maybeEmitCypressInCypress('mocha', 'test:after:run', args[0]) | ||
|
||
if (this.config('isTextTerminal')) { | ||
if (isTextTerminal) { | ||
// needed for calculating wallClockDuration | ||
// and the timings of after + afterEach hooks | ||
return this.emit('mocha', 'test:after:run', args[0]) | ||
|
@@ -628,7 +575,7 @@ class $Cypress { | |
return // do not emit hidden logs to public apis | ||
} | ||
|
||
this.runner?.addLog(args[0], this.config('isInteractive')) | ||
this.runner?.addLog(args[0], isInteractive) | ||
|
||
return this.emit('log:added', ...args) | ||
|
||
|
@@ -641,7 +588,7 @@ class $Cypress { | |
|
||
// Cypress logs will only trigger an update every 4 ms so there is a | ||
// chance the runner has been torn down when the update is triggered. | ||
this.runner?.addLog(args[0], this.config('isInteractive')) | ||
this.runner?.addLog(args[0], isInteractive) | ||
|
||
return this.emit('log:changed', ...args) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -117,15 +117,25 @@ const testAfterRun = (test, Cypress) => { | |
// perf loop only through | ||
// a tests OWN properties and not | ||
// inherited properties from its shared ctx | ||
for (let key of Object.keys(test.ctx || {})) { | ||
const value = test.ctx[key] | ||
|
||
if (_.isObject(value) && !mochaCtxKeysRe.test(key)) { | ||
// nuke any object properties that come from | ||
// cy.as() aliases or anything set from 'this' | ||
// so we aggressively perform GC and prevent obj | ||
// ref's from building up | ||
test.ctx[key] = undefined | ||
const ctx = test.ctx | ||
|
||
if (ctx) { | ||
Comment on lines
+120
to
+122
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably not super impactful, but technically more performance below 🤷🏻♀️ |
||
// Cache the regex test result and use more efficient iteration | ||
const keys = Object.keys(ctx) | ||
const keysLength = keys.length | ||
|
||
for (let i = 0; i < keysLength; i++) { | ||
const key = keys[i] | ||
const value = ctx[key] | ||
|
||
// Cache the object check and regex test for better performance | ||
if (value && typeof value === 'object' && !mochaCtxKeysRe.test(key)) { | ||
// nuke any object properties that come from | ||
// cy.as() aliases or anything set from 'this' | ||
// so we aggressively perform GC and prevent obj | ||
// ref's from building up | ||
ctx[key] = undefined | ||
} | ||
} | ||
} | ||
|
||
|
@@ -776,11 +786,18 @@ const normalize = (runnable, tests, initialTests, getRunnableId, getHookId, getO | |
} | ||
|
||
const onlyIdMode = () => { | ||
return !!getOnlyTestId() || !!getOnlySuiteId() | ||
// Cache the results to avoid repeated function calls | ||
const onlyTestId = getOnlyTestId() | ||
const onlySuiteId = getOnlySuiteId() | ||
Comment on lines
+789
to
+790
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. caching some function calls below, but not sure how impactful to performance |
||
|
||
return !!onlyTestId || !!onlySuiteId | ||
} | ||
|
||
const suiteHasOnlyId = (suite) => { | ||
return suiteHasTest(suite, getOnlyTestId()) || suiteHasSuite(suite, getOnlySuiteId()) | ||
const onlyTestId = getOnlyTestId() | ||
const onlySuiteId = getOnlySuiteId() | ||
|
||
return suiteHasTest(suite, onlyTestId) || suiteHasSuite(suite, onlySuiteId) | ||
} | ||
|
||
const normalizedRunnable = normalizeRunnable(runnable) | ||
|
@@ -809,11 +826,14 @@ const normalize = (runnable, tests, initialTests, getRunnableId, getHookId, getO | |
// recursively iterate and normalize all other _runnables | ||
_.each({ tests: runnableTests, suites: runnableSuites }, (_runnables, type) => { | ||
if (runnable[type]) { | ||
const isOnlyIdMode = onlyIdMode() | ||
const onlyTestId = isOnlyIdMode ? getOnlyTestId() : null | ||
|
||
return normalizedRunnable[type] = _.compact(_.map(_runnables, (childRunnable) => { | ||
const normalizedChild = normalize(childRunnable, tests, initialTests, getRunnableId, getHookId, getOnlyTestId, getOnlySuiteId, createEmptyOnlyTest) | ||
|
||
if (type === 'tests' && onlyIdMode()) { | ||
if (normalizedChild.id === getOnlyTestId()) { | ||
if (type === 'tests' && isOnlyIdMode) { | ||
if (normalizedChild.id === onlyTestId) { | ||
runnable.tests = [childRunnable] | ||
runnable._onlyTests = [childRunnable] | ||
|
||
|
@@ -823,8 +843,11 @@ const normalize = (runnable, tests, initialTests, getRunnableId, getHookId, getO | |
return null | ||
} | ||
|
||
if (type === 'suites' && onlyIdMode()) { | ||
if (suiteHasOnlyId(childRunnable)) { | ||
if (type === 'suites' && isOnlyIdMode) { | ||
// Cache suiteHasOnlyId result to avoid repeated calculations | ||
const hasOnlyId = suiteHasOnlyId(childRunnable) | ||
|
||
if (hasOnlyId) { | ||
runnable.suites = [childRunnable] | ||
runnable._onlySuites = [childRunnable] | ||
|
||
|
@@ -849,8 +872,10 @@ const normalize = (runnable, tests, initialTests, getRunnableId, getHookId, getO | |
const filterOnly = (normalizedSuite, suite) => { | ||
if (suite._onlyTests.length) { | ||
const suiteOnlyTests = suite._onlyTests | ||
const onlyTestId = getOnlyTestId() | ||
const hasOnlyTestId = !!onlyTestId | ||
|
||
if (getOnlyTestId()) { | ||
if (hasOnlyTestId) { | ||
suite.tests = [] | ||
suite._onlyTests = [] | ||
suite._afterAll = [] | ||
|
@@ -862,8 +887,8 @@ const normalize = (runnable, tests, initialTests, getRunnableId, getHookId, getO | |
normalizedSuite.tests = _.compact(_.map(suiteOnlyTests, (test) => { | ||
const normalizedTest = normalizeRunnable(test) | ||
|
||
if (getOnlyTestId()) { | ||
if (normalizedTest.id === getOnlyTestId()) { | ||
if (hasOnlyTestId) { | ||
if (normalizedTest.id === onlyTestId) { | ||
suite.tests = [test] | ||
suite._onlyTests = [test] | ||
|
||
|
@@ -898,12 +923,17 @@ const normalize = (runnable, tests, initialTests, getRunnableId, getHookId, getO | |
|
||
suite.suites = [] | ||
|
||
const isOnlyIdMode = onlyIdMode() | ||
|
||
normalizedSuite.suites = _.compact(_.map(suiteSuites, (childSuite) => { | ||
const normalizedChildSuite = normalize(childSuite, tests, initialTests, getRunnableId, getHookId, getOnlyTestId, getOnlySuiteId, createEmptyOnlyTest) | ||
|
||
if ((suite._onlySuites.indexOf(childSuite) !== -1) || filterOnly(normalizedChildSuite, childSuite)) { | ||
if (onlyIdMode()) { | ||
if (suiteHasOnlyId(childSuite)) { | ||
if (isOnlyIdMode) { | ||
// Cache suiteHasOnlyId result | ||
const hasOnlyId = suiteHasOnlyId(childSuite) | ||
|
||
if (hasOnlyId) { | ||
suite.suites.push(childSuite) | ||
|
||
return normalizedChildSuite | ||
|
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our initial blank page and the test isolation page that shows in between each test is just a string, HTML string. This can be cached upfront so we don't generate this string over and over. Likely not a huge win, but this empty screen can be shown a lot with a lot of tests.