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

Change Detection: Add a test case for post trashing. #18290

merged 4 commits into from Nov 7, 2019


Copy link

epiqueras commented Nov 5, 2019

Follows #18275


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.


  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
@epiqueras epiqueras added this to the Future milestone Nov 5, 2019
@epiqueras epiqueras requested review from ajitbohra, nerrad, ntwb and talldan as code owners Nov 5, 2019
@epiqueras epiqueras self-assigned this Nov 5, 2019
Copy link

aduth left a comment

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
    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( '' );

// Check that the dialog didn't show.
await assertIsDirty( false );

This comment has been minimized.

Copy link

aduth Nov 5, 2019


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 assert false.

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.

This comment has been minimized.

Copy link

epiqueras Nov 5, 2019

Author Contributor

True! I missed that.


This comment has been minimized.

Copy link
Contributor Author

epiqueras commented Nov 5, 2019


epiqueras and others added 2 commits Nov 6, 2019
Co-Authored-By: Andrew Duthie <>
@epiqueras epiqueras requested a review from gziolo as a code owner Nov 6, 2019
@karmatosed karmatosed added this to Inbox in Tightening Up Nov 6, 2019
aduth approved these changes Nov 7, 2019
Copy link

aduth left a comment


@epiqueras epiqueras merged commit e1fb88e into master Nov 7, 2019
1 of 2 checks passed
1 of 2 checks passed
Travis CI - Pull Request Build Errored
@epiqueras epiqueras deleted the add/change-detection-trashing-test-case branch Nov 7, 2019
@youknowriad youknowriad modified the milestones: Future, Gutenberg 6.9 Nov 11, 2019
CreativeDive added a commit to CreativeDive/gutenberg that referenced this pull request Nov 12, 2019
* Change Detection: Add a test case for post trashing.

* Change Detection: Fix the way redirection is asserted.

* Update packages/e2e-tests/specs/editor/various/change-detection.test.js

Co-Authored-By: Andrew Duthie <>

* Change Detection: Fix test race condition.
@karmatosed karmatosed moved this from Inbox to Ready to create in Tightening Up Nov 13, 2019
@karmatosed karmatosed moved this from Ready to create to Waiting (update, decision, block) in Tightening Up Nov 13, 2019
@karmatosed karmatosed moved this from Waiting (update, decision, block) to Done in Tightening Up Nov 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.