-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Change Detection: Add a test case for post trashing. #18290
Conversation
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.
There's a (likely related) end-to-end test failure:
FAIL packages/e2e-tests/specs/editor/various/change-detection.test.js (60.563s)
● Change detection › should not prompt to confirm unsaved changes when trashing an existing post
expect(jest.fn()).not.toHaveErrored(expected)
Expected mock function not to be called but it was called with:
["Failed to load resource: the server responded with a status of 500 (Internal Server Error)"]
at Object.expect (../jest-console/build/@wordpress/jest-console/src/index.js:34:4)
at runMicrotasks (<anonymous>)
await page.waitForSelector( '.editor-post-saved-state.is-saved' ); | ||
|
||
// Check that the dialog didn't show. | ||
await assertIsDirty( false ); |
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.
From your comment at #18275 (comment) :
This works because refreshing a page with a dialog doesn't get rid of the dialog. So if navigation didn't happen because the dialog stopped it,
assertIsDirty
wouldn't assertfalse
.
But the 'dialog'
event handler isn't added until assertIsDirty
is called, so if the dialog was already present by the time we reach this line, the event callback would never be triggered.
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.
True! I missed that.
Fixed! |
packages/e2e-tests/specs/editor/various/change-detection.test.js
Outdated
Show resolved
Hide resolved
packages/e2e-tests/specs/editor/various/change-detection.test.js
Outdated
Show resolved
Hide resolved
Co-Authored-By: Andrew Duthie <[email protected]>
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.
👍
Follows #18275
Description
This PR fixes and reintroduces the test case from #18275.
It also replaces some ad-hoc saving code with the correct e2e test util,
saveDraft
.How has this been tested?
The new tests were verified to test for the issue fixed in #18275.
Checklist: