-
Notifications
You must be signed in to change notification settings - Fork 116
fix(ie): Use 32-bit version by default for IEDriver #181
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
closes #180
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -122,7 +122,11 @@ export class FileManager { | |
} | ||
// if the suffix does not match the executable, | ||
// the binary is something like: .exe and .zip | ||
else if (existFile.indexOf(binary.suffix(osType, arch)) === -1) { | ||
// TODO(cnishina): fix implementation. Suffix method is dependent on the version number | ||
// example: chromedriver < 2.23 has a different suffix than 2.23+ (mac32.zip vs mac64.zip). | ||
else if ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this change here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is an extra check... current implementation of this needs some fixing. This is because downloading a file before 2.23 suffix is mac32.zip and this method is returning mac64.zip. Unfortunately, when we get the suffix the version is not set. Not having extra checks:
Adding a TODO(cnishina) to fix this. |
||
!existFile.endsWith('.zip') && !existFile.endsWith('.tar.gz') && | ||
existFile.indexOf(binary.suffix(osType, arch)) === -1) { | ||
editExistFile = editExistFile.replace(binary.executableSuffix(osType), ''); | ||
editExistFile = editExistFile.indexOf('_') === 0 ? | ||
editExistFile.substring(1, editExistFile.length) : | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,8 +18,6 @@ describe('status', () => { | |
// geckodriver {{config version}} [last] [default] | ||
// standalone 2.24 [last], {{config version}} [default] | ||
beforeAll((done) => { | ||
Config.osType_ = 'Linux'; | ||
Config.osArch_ = 'x64'; | ||
argv = { | ||
'_': ['update'], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why this change? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It does not matter which os arch and type we are downloading. |
||
'versions': {'chrome': '2.24', 'standalone': '2.44.0'}, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why two variables if they're just gonna be the same thing anyway? Combine both into
ie32