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 two bugs in #decompress_file. #114

Merged

Conversation

kapoorlakshya
Copy link
Collaborator

@kapoorlakshya kapoorlakshya commented May 11, 2019

This PR addresses two issues in System#decompress_file that Appveyor caught while running the screen-recorder specs (log):

  1. Undefined variable url is referenced instead of file_name when raising an error. Seems like a copy-paste error to me :).
  2. When a custom install_dir relative to the project root is used instead of the default one, target resolves to the install_dir + the driver binary. For example, ..\webdrivers_bin\chromedriver.exe. This causes a problem on Windows where the absolute path now resolves to D:\<project>\webdrivers_bin\webdrivers_bin\chromedriver.exe instead of D:\<project>\webdrivers_bin\chromedriver.exe, and File.exist? returns false for this.
> Webdrivers.install_dir
=> "webdrivers_bin"

> target
=> "webdrivers_bin/chromedriver.exe"

 File.exist? target
=> false

> File.absolute_path target
=> "D:/Projects/screen-recorder/webdrivers_bin/webdrivers_bin/chromedriver.exe"

The fix is to check for the basename (*.exe) instead of the full target path. This works because Dir.pwd is the webdrivers install dir when this check is performed in System#decomress_file.

Dir.pwd
=> "D:/Projects/screen-recorder/webdrivers_bin"

File.exist? File.basename(target)
=> true

@titusfortner titusfortner merged commit f9e67f0 into titusfortner:master May 13, 2019
@kapoorlakshya kapoorlakshya deleted the fix_decompress_file_bug branch May 14, 2019 05:02
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.

2 participants