Skip to content

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

Merged
epiqueras merged 4 commits intomasterfrom
add/change-detection-trashing-test-case
Nov 7, 2019
Merged

Change Detection: Add a test case for post trashing.#18290
epiqueras merged 4 commits intomasterfrom
add/change-detection-trashing-test-case

Conversation

@epiqueras
Copy link
Copy Markdown
Contributor

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:

  • 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 [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. [Package] Editor /packages/editor labels Nov 5, 2019
@epiqueras epiqueras added this to the Future milestone Nov 5, 2019
@epiqueras epiqueras self-assigned this Nov 5, 2019
Copy link
Copy Markdown
Member

@aduth aduth left a 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 );
Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True! I missed that.

@epiqueras
Copy link
Copy Markdown
Contributor Author

Fixed!

@epiqueras epiqueras requested a review from gziolo as a code owner November 6, 2019 18:26
Copy link
Copy Markdown
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@epiqueras epiqueras merged commit e1fb88e into master Nov 7, 2019
@epiqueras epiqueras deleted the add/change-detection-trashing-test-case branch November 7, 2019 22:51
@youknowriad youknowriad modified the milestones: Future, Gutenberg 6.9 Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Package] Editor /packages/editor [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants