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

feat(cli): better error message for ChromeDriver version mismatch #680

Conversation

not-my-profile
Copy link
Contributor

Fixes #679.

@not-my-profile not-my-profile requested a review from a team as a code owner March 6, 2023 18:53
@CLAassistant
Copy link

CLAassistant commented Mar 6, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@straker straker left a comment

Choose a reason for hiding this comment

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

Thanks for the pr! This type of message will help a ton.

packages/cli/src/bin/index.ts Outdated Show resolved Hide resolved
@michael-siek
Copy link
Member

Hey @not-my-profile just reaching out to see if you've seen the request changes on the PR.

@not-my-profile not-my-profile force-pushed the better-error-for-chromedriver-version-mismatch branch from 63c7d4e to d54a370 Compare June 30, 2023 19:13
@not-my-profile not-my-profile force-pushed the better-error-for-chromedriver-version-mismatch branch from d54a370 to aefecb3 Compare July 2, 2023 07:58
$ npm install -g chromedriver@<version>
$ axe --chromedriver-path $(npm root -g)/chromedriver/bin/chromedriver <url...>

(where <version> is the first number of the "current browser version" reported above.)`);
Copy link
Contributor

Choose a reason for hiding this comment

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

"the first number" is a bit ambiguous. We should be very specific about what we want as browser version 114.0.5735.198 the first number would be 1 which would not be the version of chromedriver we want.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're confusing "digit" with "number". I changed your suggestion of "major version" to "first number of the version" since I think there's a good chance that the user isn't familiar with SemVer.

@Zidious
Copy link
Contributor

Zidious commented Aug 7, 2023

Hey @not-my-profile !

Just missing a test and I think this is good to go

@straker
Copy link
Contributor

straker commented Aug 22, 2023

Thanks for the contribution.

Reviewed for security

@straker straker merged commit 10cf350 into dequelabs:develop Aug 22, 2023
25 of 26 checks passed
@straker straker mentioned this pull request Sep 6, 2023
1 task
@axe-core axe-core mentioned this pull request Oct 12, 2023
@github-actions github-actions bot mentioned this pull request Oct 12, 2023
@dequejenn dequejenn mentioned this pull request Oct 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cli: Provide better error message for ChromeDriver version mismatch
5 participants