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

fix(webdriver): browser request should use btoa for basic auth, not atob #7401

Merged
merged 1 commit into from
Sep 14, 2021

Conversation

jlipps
Copy link
Contributor

@jlipps jlipps commented Sep 8, 2021

Oops :-) I thought "atob" meant "ascii to base64", but it does not!

Copy link
Member

@christian-bromann christian-bromann left a comment

Choose a reason for hiding this comment

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

Can we add a test for this?

@jlipps
Copy link
Contributor Author

jlipps commented Sep 9, 2021

Hmm, yes, I think we can add a test for this, but only one that mocks atob in the first place, since it's not a Node function, and we don't have a test harness that bundles and runs code in a browser context.

Is a test that mocks atob any better than no test at all? I suppose the most it would do is make sure we don't change it to some other function name in the future?

@christian-bromann
Copy link
Member

christian-bromann commented Sep 10, 2021

@jlipps you can run Jest tests within a browser environment using jsdom, similar we do this e.g. with the isFocused test. It has a window global which has atob as property.

@jlipps
Copy link
Contributor Author

jlipps commented Sep 10, 2021

Oh, sweet! Done.

@jlipps
Copy link
Contributor Author

jlipps commented Sep 10, 2021

the windows CI failure seems unrelated.

Copy link
Member

@christian-bromann christian-bromann left a comment

Choose a reason for hiding this comment

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

This looks good, thanks 👍

@christian-bromann christian-bromann added the PR: Bug Fix 🐛 PRs that contain bug fixes label Sep 14, 2021
@christian-bromann christian-bromann merged commit e8506b1 into webdriverio:main Sep 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Bug Fix 🐛 PRs that contain bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants