-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
isolated runner changes #6799
isolated runner changes #6799
Conversation
Thanks for the contribution! Below are some guidelines Cypress uses when doing PR reviews.
PR Review ChecklistIf any of the following requirements can't be met, leave a comment in the review selecting 'Request changes', otherwise 'Approve'. User Experience
Functionality
Maintainability
Quality
Internal
|
Test summaryRun details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
ugh, this edit: needed to clear |
d1bc9ea
to
8031a43
Compare
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.
Is there a reason not to merge this into develop
branch?
TODO: Running tests in runner
needs instructions in the README.md
, right now it only describes how to run unit tests - does not describe how to start server and run cypress tests.
I'm a little unclear about the overlap of these tests versus the reporter/cypress/integration
tests. Are these meant to replace those entirely? Cause some of the things they're tested seem duplicated. Maybe I'm wrong here though.
…erna' into chore-move-driver-tests-into-parent
…tests-into-parent
…tests-into-parent
774249e
to
3f4d7df
Compare
// disable DOM snapshots during log for this agent | ||
agent.snapshot = (bool = true) => { | ||
agent._noSnapshot = !bool | ||
|
||
return agent | ||
} | ||
|
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.
might be a better / more readable API to do...
agent.log = (options) => {
if (options && options.snapshot === false) {
// do the thing
}
})
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.
Or more like agent.options
or agent.logOptions
packages/runner/cypress.json
Outdated
"hosts": { | ||
"*.foobar.com": "127.0.0.1" | ||
} |
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.
remove, copypasta fail
@@ -32,6 +32,12 @@ const socketRerunEvents = 'runner:restart watched:file:changed'.split(' ') | |||
const localBus = new EventEmitter() | |||
const reporterBus = new EventEmitter() | |||
|
|||
if (window.Cypress) { | |||
window.channel = ws |
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.
This is already exposed via line 19
Also we may just want to nest these underneath window.runnerWs
(and rename that)
window.eventManager = { backendBus, reporterBus, localBus }
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.
let's add a note reflecting on why this is a weird shape - that we only want to expose this within the test environment, but because we use an e2e test that uses it, we either need to move that test to isolated-runner or find a 2nd way to expand the conditional.
} | ||
|
||
describe('src/cypress/runner', () => { | ||
describe('tests finish with correct state', () => { |
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.
runner.mochaEvents.spec
move all the tests that snapshot mocha event structures to their own file
@@ -0,0 +1,54 @@ | |||
/** |
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.
move this to a separate draft PR
.html(btnIcon.html().replace(/\n/g, '<br/>')) | ||
}, | ||
click () { | ||
const prev = Cypress.env('SNAPSHOT_UPDATE') |
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.
don't use Cypress.env
, just put this on a global since we intend to move away from the current Cypress.env
implementation
}, function () { | ||
}) |
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.
lol 5th argument
@@ -1,60 +1,369 @@ | |||
exports['lib/reporter #stats has reporterName stats, reporterStats, etc 1'] = { | |||
exports['simple_single_test runner emit'] = [ |
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.
move to draft PR
packages/server/test/matchDeep.js
Outdated
@@ -0,0 +1,124 @@ | |||
const _ = require('lodash') |
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.
move to draft PBR
@@ -0,0 +1,92 @@ | |||
/* eslint-disable prefer-rest-params */ |
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.
move to draft PR
@@ -1,162 +1,248 @@ | |||
require('../spec_helper') |
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.
move to draft PR
* @param {()=>void | {[key:string]: any}} mochaTests | ||
* @param {{state?: any, config?: any}} opts | ||
*/ | ||
const runCypress = (mochaTests, opts = {}) => { |
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.
runIsolatedCypress
Closes N/A (internal only)
chunk of feat: support test retries #3968
cleanup cy/Cypress globals to allow testing cypress-in-cypress
add
runner/integration - runner.spec.js
that tests cypress runner as the AUTadd
server/unit - reporter.spec.js
that tests reporter from event snapshot data generated inrunner/integration - runner.spec.js
Potential Tests to move into runner/integration
User facing changelog
N/A
Additional details
isolated runner:
yes:
no:
misc:
implementation:
packages/runner/cypress/integration
How has the user experience changed?
Internal Only
added snapshot/diff utility with helpful diff error message
which is the result of this diff
update snapshots by toggling this ui button
PR Tasks