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

Testing: Add partial support for Axe verification in e2e tests #15018

Merged
merged 7 commits into from
May 6, 2019

Conversation

gziolo
Copy link
Member

@gziolo gziolo commented Apr 17, 2019

Description

This is a follow-up for #13241.

This PR enables Axe tests for every individual e2e test containing the block editor. They will run at the end of each test and perform static accessibility related analysis based on the DOM present in the test. It isn't perfect but it should help to catch basic regressions moving forward.

To get use there I had to temporary disable the following rules:

  • aria-allowed-role
  • aria-valid-attr-value
  • button-name
  • color-contrast
  • dlitem
  • duplicate-id
  • label
  • link-name
  • listitem
  • region

All rules are listed here:
https://github.com/dequelabs/axe-core/blob/develop/doc/rule-descriptions.md

I also excluded HTML elements containing the following class names:

  • .edit-post-layout__metaboxes - ignores elements created by metaboxes
  • .mce-container - gnores elements created by TinyMCE

This helps to avoid false negatives when code unrelated to Gutenberg triggers errors.

Duration of tests

There is a noticable impact on the duration of the full test suite run. I'm observing 2-5 minutes duration increase which is the cost of running verifications for all tests.

Before

Screen Shot 2019-04-18 at 13 15 34

After

Screen Shot 2019-04-18 at 13 16 16

Screen Shot 2019-04-18 at 16 39 09

Testing

npm run test-e2e

To ensure that tests fail you can enable one of the rules as follows:

diff --git a/packages/e2e-tests/config/setup-test-framework.js b/packages/e2e-tests/config/setup-test-framework.js
index 368f05409..6c6ffb25b 100644
--- a/packages/e2e-tests/config/setup-test-framework.js
+++ b/packages/e2e-tests/config/setup-test-framework.js
@@ -161,7 +161,7 @@ async function runAxeTestsForBlockEditor() {
                // Temporary disabled rules to enable initial integration.
                // See: https://github.com/WordPress/gutenberg/pull/15018.
                disabledRules: [
-                       'aria-allowed-role',
+                       // 'aria-allowed-role',
                        'aria-valid-attr-value',
                        'button-name',
                        'color-contrast',

This is how errors are going to be presented:

Screen Shot 2019-04-18 at 11 51 49

Next steps

I plan to update a follow-up issues for every disabled rule with the example of failing tests and details. I'll take care of it next week.

@gziolo gziolo added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Status] In Progress Tracking issues with work in progress [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. labels Apr 17, 2019
@gziolo gziolo self-assigned this Apr 17, 2019
@gziolo gziolo force-pushed the update/enable-jest-axe-puppeteer branch from 98d1e37 to 52a4193 Compare April 17, 2019 14:43
@gziolo gziolo changed the title [WIP] Testing: Add partial support for Axe verification in e2e tests Testing: Add partial support for Axe verification in e2e tests Apr 18, 2019
@gziolo gziolo added [Type] Enhancement A suggestion for improvement. and removed [Status] In Progress Tracking issues with work in progress labels Apr 18, 2019
@gziolo
Copy link
Member Author

gziolo commented Apr 18, 2019

Observed failures on Travis which seem to be unrelated to this PR:

FAIL packages/e2e-tests/specs/embedding.test.js (81.679s)
  ● Embedding content › should transform from image to embed block when Instagram URL is pasted
    expect(jest.fn()).not.toHaveErrored(expected)
    Expected mock function not to be called but it was called with:
    ["Failed to load https://googleads.g.doubleclick.net/pagead/id: Redirect from 'https://googleads.g.doubleclick.net/pagead/id' to 'https://googleads.g.doubleclick.net/pagead/id?slf_rd=1' has been blocked by CORS policy: No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'https://www.youtube.com' is therefore not allowed access."]
      at expect (../jest-console/build/@wordpress/jest-console/src/index.js:34:4)
  ● Embedding content › should transform from image to embed block when Instagram URL is pasted
    waiting for selector ".wp-block-embed-instagram" failed: timeout 30000ms exceeded
      280 | 		await page.keyboard.type( 'https://www.instagram.com/p/Bvl97o2AK6x/' );
      281 | 		await page.keyboard.press( 'Enter' );
    > 282 | 		await page.waitForSelector( '.wp-block-embed-instagram' );
          | 		           ^
      283 | 	} );
      284 | 
      285 | 	it( 'should transform from audio to embed block when Soundcloud URL is pasted', async () => {
      at new WaitTask (../../node_modules/puppeteer/lib/FrameManager.js:840:28)
      at Frame._waitForSelectorOrXPath (../../node_modules/puppeteer/lib/FrameManager.js:731:12)
      at Frame.waitForSelector (../../node_modules/puppeteer/lib/FrameManager.js:690:17)
      at Page.waitForSelector (../../node_modules/puppeteer/lib/Page.js:1008:29)
      at Object.waitForSelector (specs/embedding.test.js:282:14)
      at tryCatch (../../node_modules/regenerator-runtime/runtime.js:62:40)
      at Generator.invoke [as _invoke] (../../node_modules/regenerator-runtime/runtime.js:288:22)
      at Generator.prototype.(anonymous function) [as next] (../../node_modules/regenerator-runtime/runtime.js:114:21)
      at asyncGeneratorStep (specs/embedding.test.js:9:103)
FAIL specs/embedding.test.js (36.507s)
  ● Embedding content › should transform from image to embed block when Instagram URL is pasted
    expect(jest.fn()).not.toHaveErrored(expected)
    Expected mock function not to be called but it was called with:
    ["Failed to load https://googleads.g.doubleclick.net/pagead/id: Redirect from 'https://googleads.g.doubleclick.net/pagead/id' to 'https://googleads.g.doubleclick.net/pagead/id?slf_rd=1' has been blocked by CORS policy: No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'https://www.youtube.com' is therefore not allowed access."]
      at expect (../jest-console/build/@wordpress/jest-console/src/index.js:34:4)

@notnownikki any ideas? It looks like there are real network requests being triggered. I don't observe those failures locally.

@notnownikki
Copy link
Member

The tests look fine to me, all requests should be mocked there and I don't see those network requests when running locally. Looking at it further now.

@notnownikki
Copy link
Member

This might take me a while, Gutenberg master to longer works for me locally at all.

react-dom.production.min.js:117 Error: Cannot bind shift+alt+n. Alt and Shift+Alt modifiers are reserved for character input.

@notnownikki
Copy link
Member

All I can tell at the moment is that during the tests, we're getting youtube content embedded during tests that should not touch youtube at all. That's very very weird.

However, there is a network request happening inside the image block, and perhaps if we mock that, these errors might disappear. It needs to be done anyway.

The way the image block to embed block replacement is triggered is through trying to display the image with the src set to the url of what the user entered, and onError we look for a suitable embed block. That means the Instagram URL is going to be requested over the network during the test, and it should be mocked so we get a reliable, known response during the test. That's the only issue I can find so far.

@gziolo gziolo force-pushed the update/enable-jest-axe-puppeteer branch 2 times, most recently from 0f126de to d93f590 Compare April 18, 2019 14:05
@gziolo gziolo force-pushed the update/enable-jest-axe-puppeteer branch from 26ebd7d to e792d47 Compare April 30, 2019 16:01
@gziolo
Copy link
Member Author

gziolo commented Apr 30, 2019

The duration of jobs after rebase:

Screen Shot 2019-04-30 at 18 27 56


await expect( page ).toPassAxeTests( {
// Temporary disabled rules to enable initial integration.
// See: https://github.com/WordPress/gutenberg/pull/15018.
Copy link
Member

Choose a reason for hiding this comment

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

It would be good to have these in a tracking issue, perhaps structured as a checklist to iteratively mark progress.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I will do it after merging this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

#15452 opened

@gziolo gziolo force-pushed the update/enable-jest-axe-puppeteer branch from e792d47 to 21ea6ea Compare May 6, 2019 09:07
@gziolo gziolo merged commit a797ec5 into master May 6, 2019
@gziolo gziolo deleted the update/enable-jest-axe-puppeteer branch May 6, 2019 09:42
@youknowriad youknowriad added this to the 5.7 (Gutenberg) milestone May 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants