Allow multiple snapshots within a single test (Fixes #93) #96
Conversation
| @@ -20,14 +22,29 @@ jest.mock('fs-extra', () => ({ | |||
| readFileSync: () => 'cheese', | |||
| pathExistsSync: () => false, | |||
| copySync: () => null, | |||
| removeSync: () => null, | |||
| removeSync: jest.fn().mockReturnValue(null), | |||
There was a problem hiding this comment.
Could be made more terse:
| removeSync: jest.fn().mockReturnValue(null), | |
| removeSync: jest.fn(() => null), |
There was a problem hiding this comment.
I think I will leave this as-is because it is more consistent with other usages in this codebase (mockReturnValue, mockResolvedValue, etc).
|
|
||
| const screenshotsFolder = Cypress.config('screenshotsFolder'); | ||
| const updateSnapshots = Cypress.env('updateSnapshots') || false; | ||
| const failOnSnapshotDiff = | ||
| typeof Cypress.env('failOnSnapshotDiff') === 'undefined'; | ||
| typeof Cypress.env('failOnSnapshotDiff') === 'undefined' || | ||
| Cypress.env('failOnSnapshotDiff'); |
There was a problem hiding this comment.
Does this return a Boolean or a string that says true?
|
@crosscompile Any thoughts? Also - do you know what I need to do to stop Chronicler from failing this build. /cc @badeball You reported the original issue so linking you, too. |
sebinsua
force-pushed
the
fix/93-unable-to-record-multiple-snapshots-within-a-single-test
branch
from
b1f8ee9 to
12f0275
Compare
| // After completing each test, we need to instruct the plugin to clean up | ||
| // the screenshots that were created. | ||
| afterEach(() => { | ||
| cy.task(CLEAN_SCREENSHOTS, null, { log: false }); |
There was a problem hiding this comment.
@NMinhNguyen This will ensure that implementation details of our approach aren't exposed to the user.
|
@crosscompile Are you happy for this to be merged? |
|
@crosscompile @jaredpalmer Are either of you going to be able to merge and publish this? I've been waiting since August and at this point I'm starting to consider just publishing my own fork. |
|
It seems that this project may have been abandoned. :-( @sebinsua If you publish a fork, let me know. |
|
@jasonharrison Yeah, I'm not sure if/when they'll come back to this, so I've created this fork. I'm just as likely as anybody else within the community to run out of time to maintain this, so what I'll try to do is to follow the Community Continuity Guidelines and give merge access to anybody that gets a contribution in. Hopefully that way we can spread the workload. |
|
I'm sorry for not coming back to you sooner with feedback on this. I've tested the branch and can confirm that it does indeed solve the problem that we initially had. Thanks for your great work! And I hope this can get merged at some point. |
Allow multiple snapshots within a single test (Fixes #93).
I also fixed a few extra problems I saw:
eslintnow allowsjestglobals.process.cwdshould be reset even if a test fails.cypress run --env failOnSnapshotDiff=trueshould resolve totrue(previously the code only consideredundefinedto betrue).