feat: upgrade to axe-core 3.0.0-alpha.9 #46

marcysutton · 2017-11-27T19:41:19Z

@WilcoFiers is attest-master capable of handling prerelease versions? This PR still needs a version bump and a changelog update.

marcysutton · 2017-11-27T20:41:44Z

test/integration/shadow-dom.js

+          var script = document.createElement('script');
+          script.innerHTML = 'var shadowSupport = document.body && typeof document.body.attachShadow === \'function\';';
+          document.documentElement.appendChild(script);
+        	shadowSupported = shadowSupport;


@WilcoFiers any ideas on how I can get a variable back out of executeAsyncScript? The shadowSupport check is working, but I can't store it in a variable in the outer scope.

marcysutton · 2017-12-04T22:01:37Z

@WilcoFiers @dylanb I pushed a change that updates axe-webdriverjs to use axe.run, but it's failing internally when I use executeAsyncScript in a before block. I just happened to uncover it by sniffing for Shadow DOM support in this new test. It throws a really weird error:

 1) shadow-dom.html should find violations:
     Uncaught JavascriptError: javascript error: Cannot read property 'reporter' of null
JavaScript stack:
TypeError: Cannot read property 'reporter' of null
    at Object.axe.run (<anonymous>:1475:32)
    at eval (eval at executeAsyncScript (:453:5), <anonymous>:8:16)
    at eval (eval at executeAsyncScript (:453:5), <anonymous>:10:7)
    at eval (eval at executeAsyncScript (:453:5), <anonymous>:10:33)
    at executeAsyncScript (<anonymous>:453:26)
    at <anonymous>:469:29
    at callFunction (<anonymous>:361:33)
    at <anonymous>:371:23
    at <anonymous>:372:3
  (Session info: chrome=62.0.3202.94)
  (Driver info: chromedriver=2.33.506106 (8a06c39c4582fbfbab6966dbb1c38a9173bfb1a2),platform=Mac OS X 10.11.6 x86_64)
    at WebDriverError (node_modules/selenium-webdriver/lib/error.js:27:5)
    at JavascriptError (node_modules/selenium-webdriver/lib/error.js:157:5)
    at Object.checkLegacyResponse (node_modules/selenium-webdriver/lib/error.js:546:15)
    at parseHttpResponse (node_modules/selenium-webdriver/lib/http.js:509:13)
    at doSend.then.response (node_modules/selenium-webdriver/lib/http.js:441:30)
    at process._tickDomainCallback (internal/process/next_tick.js:129:7)

  From: Task: WebDriver.executeScript()
    at thenableWebDriverProxy.schedule (node_modules/selenium-webdriver/lib/webdriver.js:807:17)
    at thenableWebDriverProxy.executeAsyncScript (node_modules/selenium-webdriver/lib/webdriver.js:891:17)
    at lib/index.js:128:5
    at lib/inject.js:59:4
    at ManagedPromise.invokeCallback_ (node_modules/selenium-webdriver/lib/promise.js:1376:14)
    at TaskQueue.execute_ (node_modules/selenium-webdriver/lib/promise.js:3084:14)
    at TaskQueue.executeNext_ (node_modules/selenium-webdriver/lib/promise.js:3067:27)
    at asyncRun (node_modules/selenium-webdriver/lib/promise.js:2927:27)
    at node_modules/selenium-webdriver/lib/promise.js:668:7
    at process._tickDomainCallback (internal/process/next_tick.js:129:7)

It seems like it's getting confused by the two async script calls (the one in my test, and the other one in lib/inject). If I move the Shadow DOM logic into the test, it passes. But I both wanted the Shadow DOM check to be reusable AND fix it in case any users are executing async scripts in their tests. This is only a problem with axe.run, not with axe.a11yCheck.

marcysutton · 2017-12-04T22:04:24Z

It's also worth noting that I think we should keep the AxeBuilder.analyze API the same, and not require users to pass an error parameter to the callback function. That way people's tests won't break.

marcysutton · 2017-12-06T23:32:50Z

Ok, the issue with executeAsyncScript has been resolved. It was failing due to options being null, it just needed to default to an empty object.

I also wrapped the body of the analyze function in a Promise so we can resolve and reject based on what axe.run does.

WilcoFiers · 2017-12-07T09:27:13Z

test/unit/index.js

+					}
+				})
+				.analyze()
+				.then()


I don't think this is needed.

The then() call? Or the entire test?

WilcoFiers · 2017-12-07T09:28:34Z

test/unit/index.js

@@ -255,5 +255,54 @@ describe('Builder', function () {
 					done();
 				});
 		});
+
+		it('should pass results to .then() instead of a callback', function (done) {


We should also have a callback that checks the analyze callback is executed before .then().

WilcoFiers · 2017-12-07T09:30:48Z

package.json

@@ -64,7 +64,7 @@
    "sinon": "^1.17.3"
  },
  "dependencies": {
-    "axe-core": "^2.5.0",
+    "axe-core": "^3.0.0-alpha.8",


I think we should release this with 2.5.0. Maybe we could use npm-install-version to install axe-core@next so we can run that shadowDOM test separately. WDYT?

Sounds like more complexity to me. But we can discuss it later.

We talked about doing a release with axe-core 2.6 and the axe.run changes, and a separate Shadow DOM prerelease later. I've separated the changes into two PRs so we can do that.

marcysutton · 2018-01-09T00:27:22Z

@WilcoFiers I've updated this PR to remove conflicts. Looks like landmark-one-main didn't make it into a 3x-alpha release, so I removed a reference to it to keep tests passing.

WilcoFiers

Circle is failing.

marcysutton · 2018-01-18T15:24:09Z

That's so odd, the previous builds on this PR didn't have the same timeout issues. I wonder if that means axe-core is significantly slower in alpha.9? That should be including the configurable script timeout, with double the default value.

marcysutton · 2018-01-18T16:25:04Z

The tests are passing now, turns out Circle didn't know how to handle ^3.0.0-alpha.9. Removing the ^ fixed it.

marcysutton requested a review from WilcoFiers November 27, 2017 19:41

marcysutton commented Nov 27, 2017

View reviewed changes

marcysutton force-pushed the update-3.0 branch from 43dab9b to f3aa36e Compare December 4, 2017 21:59

WilcoFiers reviewed Dec 7, 2017

View reviewed changes

marcysutton mentioned this pull request Dec 14, 2017

Migrate to axe.run for 2x #47

Merged

marcysutton force-pushed the update-3.0 branch from f88c2ca to 9b4f8cd Compare December 14, 2017 19:00

marcysutton force-pushed the update-3.0 branch from 9b4f8cd to 500e073 Compare January 9, 2018 00:26

WilcoFiers approved these changes Jan 9, 2018

View reviewed changes

marcysutton changed the title ~~feat: upgrade to axe-core 3.0.0-alpha.8~~ feat: upgrade to axe-core 3.0.0-alpha.9 Jan 18, 2018

WilcoFiers suggested changes Jan 18, 2018

View reviewed changes

Marcy Sutton added 6 commits January 18, 2018 17:57

feat: upgrade to axe-core 3.0.0-alpha.8

6ae3190

feat: add iframe shadow test

bccad76

Sniff for shadow-dom in tests

b972fb3

chore: fix conflicts, remove missing rule

292cd73

chore: update to axe-core@3.0.0-alpha.9

9419f01

test: extend script timeout

a5baeb2

marcysutton force-pushed the update-3.0 branch from dbfd176 to a5baeb2 Compare January 18, 2018 17:01

marcysutton merged commit 097a012 into develop Jan 19, 2018

marcysutton deleted the update-3.0 branch January 19, 2018 09:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: upgrade to axe-core 3.0.0-alpha.9 #46

feat: upgrade to axe-core 3.0.0-alpha.9 #46

marcysutton commented Nov 27, 2017

marcysutton Nov 27, 2017

marcysutton commented Dec 4, 2017 •

edited

Loading

marcysutton commented Dec 4, 2017

marcysutton commented Dec 6, 2017

WilcoFiers Dec 7, 2017

marcysutton Dec 7, 2017

WilcoFiers Dec 7, 2017

WilcoFiers Dec 7, 2017

marcysutton Dec 7, 2017

marcysutton Dec 14, 2017

marcysutton commented Jan 9, 2018

WilcoFiers left a comment

marcysutton commented Jan 18, 2018 •

edited

Loading

marcysutton commented Jan 18, 2018

feat: upgrade to axe-core 3.0.0-alpha.9 #46

feat: upgrade to axe-core 3.0.0-alpha.9 #46

Conversation

marcysutton commented Nov 27, 2017

marcysutton Nov 27, 2017

Choose a reason for hiding this comment

marcysutton commented Dec 4, 2017 • edited Loading

marcysutton commented Dec 4, 2017

marcysutton commented Dec 6, 2017

WilcoFiers Dec 7, 2017

Choose a reason for hiding this comment

marcysutton Dec 7, 2017

Choose a reason for hiding this comment

WilcoFiers Dec 7, 2017

Choose a reason for hiding this comment

WilcoFiers Dec 7, 2017

Choose a reason for hiding this comment

marcysutton Dec 7, 2017

Choose a reason for hiding this comment

marcysutton Dec 14, 2017

Choose a reason for hiding this comment

marcysutton commented Jan 9, 2018

WilcoFiers left a comment

Choose a reason for hiding this comment

marcysutton commented Jan 18, 2018 • edited Loading

marcysutton commented Jan 18, 2018

marcysutton commented Dec 4, 2017 •

edited

Loading

marcysutton commented Jan 18, 2018 •

edited

Loading