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.

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: test runner reporter performance #17243

Merged
merged 9 commits into from
Jul 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
36 changes: 20 additions & 16 deletions packages/reporter/cypress/integration/test_errors_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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({})
Expand Down
13 changes: 0 additions & 13 deletions packages/reporter/src/lib/base.scss
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,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 {
width: 10px;
height: 10px;
}

::-webkit-scrollbar-thumb {
background: #999;
}

::-webkit-scrollbar-track {
background: #DDD;
}

&,
input,
textarea {
Expand Down
2 changes: 0 additions & 2 deletions packages/reporter/src/runnables/runnable-and-suite.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
1 change: 0 additions & 1 deletion packages/reporter/src/runnables/runnable-model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<HookProps> = []
Expand Down
20 changes: 1 addition & 19 deletions packages/reporter/src/runnables/runnables-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ export class RunnablesStore {
this.hasTests = numTests > 0
this.hasSingleTest = numTests === 1

this._startRendering()
this._finishedInitialRendering()
}

_createRunnableChildren (runnableProps: RootRunnable, level: number) {
Expand Down Expand Up @@ -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
Expand Down
6 changes: 2 additions & 4 deletions packages/reporter/src/test/test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,9 @@ class Test extends Component<TestProps> {

_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) {
Expand All @@ -141,8 +141,6 @@ class Test extends Component<TestProps> {
render () {
const { model } = this.props

if (!model.shouldRender) return null

return (
<Collapsible
containerRef={this.containerRef}
Expand Down
4 changes: 2 additions & 2 deletions packages/server/test/e2e/5_screenshots_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down