From a49f61e5f6dad0d6623a0bcd5b1d6c39c0981731 Mon Sep 17 00:00:00 2001 From: Tim Griesser Date: Wed, 7 Jul 2021 22:13:25 -0400 Subject: [PATCH 1/5] fix: test runner reporter performance --- .../integration/unit/runnables_store_spec.ts | 7 ------- packages/reporter/src/lib/base.scss | 21 ++++++++++++------- packages/reporter/src/main.tsx | 1 + .../src/runnables/runnable-and-suite.tsx | 2 -- .../reporter/src/runnables/runnable-model.ts | 1 - .../reporter/src/runnables/runnables-store.ts | 20 +----------------- packages/reporter/src/test/test.tsx | 6 ++---- 7 files changed, 18 insertions(+), 40 deletions(-) diff --git a/packages/reporter/cypress/integration/unit/runnables_store_spec.ts b/packages/reporter/cypress/integration/unit/runnables_store_spec.ts index 53f2afa7fff9..e66dba0e3f97 100644 --- a/packages/reporter/cypress/integration/unit/runnables_store_spec.ts +++ b/packages/reporter/cypress/integration/unit/runnables_store_spec.ts @@ -158,13 +158,6 @@ describe('runnables store', () => { expect(instance.hasSingleTest).to.be.false }) - it('starts rendering the runnables on requestAnimationFrame', () => { - instance.setRunnables({ tests: [], suites: [createSuite('1', [], []), createSuite('2', [createTest('1')], [])] }) - expect(instance.runnables[0].shouldRender).to.be.true - expect(instance.runnables[1].shouldRender).to.be.true - expect((instance.runnables[1] as SuiteModel).children[0].shouldRender).to.be.true - }) - it('sets scrollTop when app is running and initial scrollTop has been set', () => { instance.setInitialScrollTop(234) instance.setRunnables({}) diff --git a/packages/reporter/src/lib/base.scss b/packages/reporter/src/lib/base.scss index 16a6457edd35..6b97af44f7bb 100644 --- a/packages/reporter/src/lib/base.scss +++ b/packages/reporter/src/lib/base.scss @@ -19,16 +19,23 @@ body,div,dl,dt,dd,ul,ol,li,h1,h2,h3,h4,h5,h6,pre,code,form,fieldset,legend,input } ::-webkit-scrollbar { - width: 10px; - height: 10px; + display: none; } - ::-webkit-scrollbar-thumb { - background: #999; - } + &.inactive { + ::-webkit-scrollbar { + display: unset; + width: 10px; + height: 10px; + } - ::-webkit-scrollbar-track { - background: #DDD; + ::-webkit-scrollbar-thumb { + background: #999; + } + + ::-webkit-scrollbar-track { + background: #DDD; + } } &, diff --git a/packages/reporter/src/main.tsx b/packages/reporter/src/main.tsx index dd1765fc6633..9dc30226a7c9 100644 --- a/packages/reporter/src/main.tsx +++ b/packages/reporter/src/main.tsx @@ -93,6 +93,7 @@ class Reporter extends Component { return (
diff --git a/packages/reporter/src/runnables/runnable-and-suite.tsx b/packages/reporter/src/runnables/runnable-and-suite.tsx index 66f3939057c9..53c173ada7ca 100644 --- a/packages/reporter/src/runnables/runnable-and-suite.tsx +++ b/packages/reporter/src/runnables/runnable-and-suite.tsx @@ -21,8 +21,6 @@ interface SuiteProps { } const Suite = observer(({ eventManager = events, model }: SuiteProps) => { - if (!model.shouldRender) return null - const _launchStudio = (e: MouseEvent) => { e.preventDefault() e.stopPropagation() diff --git a/packages/reporter/src/runnables/runnable-model.ts b/packages/reporter/src/runnables/runnable-model.ts index 5c6ae09d85ae..89241bef86f6 100644 --- a/packages/reporter/src/runnables/runnable-model.ts +++ b/packages/reporter/src/runnables/runnable-model.ts @@ -9,7 +9,6 @@ export interface RunnableProps { export default class Runnable { @observable id: string - @observable shouldRender: boolean = false @observable title?: string @observable level: number @observable hooks: Array = [] diff --git a/packages/reporter/src/runnables/runnables-store.ts b/packages/reporter/src/runnables/runnables-store.ts index f80065e8594e..449a545347d3 100644 --- a/packages/reporter/src/runnables/runnables-store.ts +++ b/packages/reporter/src/runnables/runnables-store.ts @@ -81,7 +81,7 @@ export class RunnablesStore { this.hasTests = numTests > 0 this.hasSingleTest = numTests === 1 - this._startRendering() + this._finishedInitialRendering() } _createRunnableChildren (runnableProps: RootRunnable, level: number) { @@ -120,24 +120,6 @@ export class RunnablesStore { return test } - // progressively renders the runnables instead of all of them being rendered - // at once. this prevents a noticeable lag in initial rendering when there - // is a large number of tests - _startRendering (index = 0) { - requestAnimationFrame(action('start:rendering', () => { - const runnable = this._runnablesQueue[index] - - if (!runnable) { - this._finishedInitialRendering() - - return - } - - runnable.shouldRender = true - this._startRendering(index + 1) - })) - } - _finishedInitialRendering () { if (this.appState.isRunning) { // have an initial scrollTop set, meaning we reloaded from a domain change diff --git a/packages/reporter/src/test/test.tsx b/packages/reporter/src/test/test.tsx index b0252ca171b9..bc3ec513e306 100644 --- a/packages/reporter/src/test/test.tsx +++ b/packages/reporter/src/test/test.tsx @@ -126,9 +126,9 @@ class Test extends Component { _scrollIntoView () { const { appState, model, scroller } = this.props - const { state, shouldRender } = model + const { state } = model - if (appState.autoScrollingEnabled && (appState.isRunning || appState.studioActive) && shouldRender && state !== 'processing') { + if (appState.autoScrollingEnabled && (appState.isRunning || appState.studioActive) && state !== 'processing') { window.requestAnimationFrame(() => { // since this executes async in a RAF the ref might be null if (this.containerRef.current) { @@ -141,8 +141,6 @@ class Test extends Component { render () { const { model } = this.props - if (!model.shouldRender) return null - return ( Date: Wed, 7 Jul 2021 23:33:39 -0400 Subject: [PATCH 2/5] fix tests --- .../cypress/integration/test_errors_spec.ts | 36 ++++++++++--------- .../server/test/e2e/5_screenshots_spec.js | 4 +-- 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/packages/reporter/cypress/integration/test_errors_spec.ts b/packages/reporter/cypress/integration/test_errors_spec.ts index 2adbeb44ad5f..0c1f6337421b 100644 --- a/packages/reporter/cypress/integration/test_errors_spec.ts +++ b/packages/reporter/cypress/integration/test_errors_spec.ts @@ -247,28 +247,12 @@ describe('test errors', () => { cy.percySnapshot() }) - it('does not show code frame when not included on error', () => { - commandErr.codeFrame = undefined - - cy - .get('.test-err-code-frame') - .should('not.exist') - }) - it('use correct language class', () => { cy .get('.test-err-code-frame pre') .should('have.class', 'language-javascript') }) - it('falls back to text language class', () => { - // @ts-ignore - commandErr.codeFrame.language = null - cy - .get('.test-err-code-frame pre') - .should('have.class', 'language-text') - }) - it('displays tooltip on hover', () => { cy.get('.test-err-code-frame a').first().trigger('mouseover') cy.get('.cy-tooltip').first().should('have.text', 'Open in IDE') @@ -285,6 +269,26 @@ describe('test errors', () => { }) }) + describe('code frames', () => { + it('does not show code frame when not included on error', () => { + commandErr.codeFrame = undefined + setError(commandErr) + + cy + .get('.test-err-code-frame') + .should('not.exist') + }) + + it('falls back to text language class', () => { + // @ts-ignore + commandErr.codeFrame.language = null + setError(commandErr) + cy + .get('.test-err-code-frame pre') + .should('have.class', 'language-text') + }) + }) + describe('studio error', () => { beforeEach(() => { setError(runnablesWithErr) diff --git a/packages/server/test/e2e/5_screenshots_spec.js b/packages/server/test/e2e/5_screenshots_spec.js index de3948aa46ea..be612fa64333 100644 --- a/packages/server/test/e2e/5_screenshots_spec.js +++ b/packages/server/test/e2e/5_screenshots_spec.js @@ -98,8 +98,8 @@ describe('e2e screenshots', () => { // make sure all of the values are unique expect(sizes).to.deep.eq(_.uniq(sizes)) - // png1 should not be within 3k of png2 - expect(sizes[0]).not.to.be.closeTo(sizes[1], 3000) + // png1 should not be within 1k of png2 + expect(sizes[0]).not.to.be.closeTo(sizes[1], 1000) }).then(() => { return Promise.all([ sizeOf(screenshot1), From 00c6d5f33db9c79346e412d4e3ba550e90e42591 Mon Sep 17 00:00:00 2001 From: Tim Griesser Date: Fri, 9 Jul 2021 07:46:33 -0400 Subject: [PATCH 3/5] Restore scrollbar --- packages/reporter/src/lib/base.scss | 20 -------------------- packages/reporter/src/main.tsx | 1 - 2 files changed, 21 deletions(-) diff --git a/packages/reporter/src/lib/base.scss b/packages/reporter/src/lib/base.scss index 6b97af44f7bb..64dfb1615c3e 100644 --- a/packages/reporter/src/lib/base.scss +++ b/packages/reporter/src/lib/base.scss @@ -18,26 +18,6 @@ body,div,dl,dt,dd,ul,ol,li,h1,h2,h3,h4,h5,h6,pre,code,form,fieldset,legend,input box-sizing: border-box; } - ::-webkit-scrollbar { - display: none; - } - - &.inactive { - ::-webkit-scrollbar { - display: unset; - width: 10px; - height: 10px; - } - - ::-webkit-scrollbar-thumb { - background: #999; - } - - ::-webkit-scrollbar-track { - background: #DDD; - } - } - &, input, textarea { diff --git a/packages/reporter/src/main.tsx b/packages/reporter/src/main.tsx index 9dc30226a7c9..dd1765fc6633 100644 --- a/packages/reporter/src/main.tsx +++ b/packages/reporter/src/main.tsx @@ -93,7 +93,6 @@ class Reporter extends Component { return (
From f1d621b0586df9b1561edbef66edb06078ef727f Mon Sep 17 00:00:00 2001 From: Tim Griesser Date: Fri, 9 Jul 2021 16:50:17 -0400 Subject: [PATCH 4/5] Attempt to add tests for regressions --- packages/server/test/e2e/7_perf_spec.ts | 9 +++++++++ .../projects/e2e/cypress/integration/perf_spec.js | 13 +++++++++++++ 2 files changed, 22 insertions(+) create mode 100644 packages/server/test/e2e/7_perf_spec.ts create mode 100644 packages/server/test/support/fixtures/projects/e2e/cypress/integration/perf_spec.js diff --git a/packages/server/test/e2e/7_perf_spec.ts b/packages/server/test/e2e/7_perf_spec.ts new file mode 100644 index 000000000000..8521441b3470 --- /dev/null +++ b/packages/server/test/e2e/7_perf_spec.ts @@ -0,0 +1,9 @@ +import e2e from '../support/helpers/e2e' + +describe('performance test', () => { + e2e.it('perf check', { + spec: 'perf_spec.js', + timeout: 60000, + headed: true, + }) +}) diff --git a/packages/server/test/support/fixtures/projects/e2e/cypress/integration/perf_spec.js b/packages/server/test/support/fixtures/projects/e2e/cypress/integration/perf_spec.js new file mode 100644 index 000000000000..a5556d35dcf4 --- /dev/null +++ b/packages/server/test/support/fixtures/projects/e2e/cypress/integration/perf_spec.js @@ -0,0 +1,13 @@ +Cypress.config().isInteractive = false + +describe('executes within a reasonable timeframe', function () { + this.timeout(5000) + + const COUNT = 200 + + Cypress._.times(COUNT, (n) => { + it(`${n} test`, () => { + expect(true).to.be.true + }) + }) +}) From 9aa2d46ecff21266d9e923c714d0c99cfb937b67 Mon Sep 17 00:00:00 2001 From: Tim Griesser Date: Fri, 9 Jul 2021 21:27:35 -0400 Subject: [PATCH 5/5] Undo perf spec --- packages/server/test/e2e/7_perf_spec.ts | 9 --------- .../projects/e2e/cypress/integration/perf_spec.js | 13 ------------- 2 files changed, 22 deletions(-) delete mode 100644 packages/server/test/e2e/7_perf_spec.ts delete mode 100644 packages/server/test/support/fixtures/projects/e2e/cypress/integration/perf_spec.js diff --git a/packages/server/test/e2e/7_perf_spec.ts b/packages/server/test/e2e/7_perf_spec.ts deleted file mode 100644 index 8521441b3470..000000000000 --- a/packages/server/test/e2e/7_perf_spec.ts +++ /dev/null @@ -1,9 +0,0 @@ -import e2e from '../support/helpers/e2e' - -describe('performance test', () => { - e2e.it('perf check', { - spec: 'perf_spec.js', - timeout: 60000, - headed: true, - }) -}) diff --git a/packages/server/test/support/fixtures/projects/e2e/cypress/integration/perf_spec.js b/packages/server/test/support/fixtures/projects/e2e/cypress/integration/perf_spec.js deleted file mode 100644 index a5556d35dcf4..000000000000 --- a/packages/server/test/support/fixtures/projects/e2e/cypress/integration/perf_spec.js +++ /dev/null @@ -1,13 +0,0 @@ -Cypress.config().isInteractive = false - -describe('executes within a reasonable timeframe', function () { - this.timeout(5000) - - const COUNT = 200 - - Cypress._.times(COUNT, (n) => { - it(`${n} test`, () => { - expect(true).to.be.true - }) - }) -})