dequelabs / axe-webdriverjs

Provides a chainable axe API for Selenium's WebDriverJS and automatically injects into all frames.
Mozilla Public License 2.0
130 stars 46 forks source link

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

Closed marcysutton closed 6 years ago

marcysutton commented 6 years ago

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

marcysutton commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

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.

marcysutton commented 6 years ago

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

marcysutton commented 6 years ago

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 commented 6 years ago

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