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

Axe cli update #18

Closed WilcoFiers closed 7 years ago

WilcoFiers commented 7 years ago

Update that will enable running axe-cli (soon to be released).

Changes include:

CLAassistant commented 7 years ago

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 4 committers have signed the CLA.

:white_check_mark: marcysutton
:x: Wilco Fiers
:x: dylanb
:x: iandotkelly


Wilco Fiers seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.

marcysutton commented 7 years ago

@WilcoFiers what are all of these other commits?

marcysutton commented 7 years ago

@WilcoFiers @dylanb there are a ton of old commits, I assume from Bitbucket, included in this PR. How do we resolve the CLA for Sturley's account since he has commits in there? Perhaps we should limit this PR to only the things relevant to axe-cli instead of the entire history?

WilcoFiers commented 7 years ago

I don't much like that. There is a bunch of good tests in here that we should be sharing. Can we rebase to get around the issue?

dylanb commented 7 years ago

Ignore the CLA issues : these are all Deque employees/contractors where we have signed agreements on IP

marcysutton commented 7 years ago

@WilcoFiers I'm not sure where you'd start a rebase from–some of the older commits from Bitbucket don't exist in this repo, while some represent content that already exists (like Contributing.md). The older commits didn't seem critical to this PR, but untangling them would be more trouble than it's worth. The merge conflicts look pretty easy to resolve.

dylanb commented 7 years ago

if we don't need to worry about the CLA issues, then what reason is there left to rebase at all? Just fix the merge conflicts.

marcysutton commented 7 years ago

I rebased on this repo to resolve the conflicts. The configure API method was missed somehow, so I added it back in. But the tests do not pass with Selenium Webdriver 3.

marcysutton commented 7 years ago

I still can't run the sauce integration test–but otherwise this PR works. I think we should remove that one but I'll leave it for another day.

@WilcoFiers can you address @dylanb's comment about the axe source and then I'll merge?

WilcoFiers commented 7 years ago

@marcysutton @dylanb I updated the way axe gets loaded in. I'm not sure why we had such complicated code. All we needed to do was this:

axeSource = axeSource || require('axe-core').source