samnissen / watir-screenshot-stitch

Extends Watir to take full page screenshots of websites.
MIT License
17 stars 7 forks source link

File path utility does not account for bundler path options #54

Closed samnissen closed 4 years ago

samnissen commented 4 years ago

Per the bundler documentation, the gem path might be different from our accounted for location:

path (BUNDLE_PATH): The location on disk where all gems in your bundle will be located regardless of $GEM_HOME or $GEM_PATH values. Bundle gems not found in this location will be installed by bundle install. Defaults to Gem.dir. When --deployment is used, defaults to vendor/bundle.

Unfortunately, the documentation also allows for many other options — including setting it on the fly during a bundle install.

Flags passed to bundle install or the Bundler runtime, such as --path foo or --without production, are not remembered between commands. If these options must be remembered, they must be set using bundle config (e.g., bundle config path foo).

Because of this I am not sure we can account for all options. However, we should be able to search additional directories, including:

Bundler's --deployment default of APP_ROOT/vendor/bundle might be discoverable as well, especially in Rails projects where we can call Rails.root. How to find this in non-Rails projects will require additional investigation.

Also, we can setup some custom way of alerting us to the gem's path (i.e. WATIR_SCREENSHOT_STITCH_GEM_DIRECTORY).

Since this is a destructive act, we can provide a backstop of employing gem which watir-screenshot-stitch. This could mean version mismatch (and thus unexpected results), and we might want to warn users of that if they reach such a code path.

At a minimum, we must document this dependency for anyone using the base64_canvas function.

samnissen commented 4 years ago

Would like to know if you think this is worth tackling, @sandeepnagra , or if you have other ideas about how we can ensure we can access our own lib directory.

sandeepnagra commented 4 years ago

Hi @samnissen, Apologies for the delayed response. I have been a little busy and just got some time today to look into it.

I have an alternate to load the path for html2canvas js files that works for me. I use this logic in several other gems to load files in gem but all my gems are used in non-rails projects. Here's what I did:

I replaced following: path = File.join(WatirScreenshotStitch::Utilities.directory, "vendor/html2canvas.js") with: path = File.expand_path("../../vendor/html2canvas.js", __FILE__)

Tests are executing fine. I can raise a PR for this and also remove WatirScreenshotStitch::Utilities as we don't need it anymore.

Try it in your local and let me know if it doesn't work.

samnissen commented 4 years ago

No reason to apologize. And good work all around. Just did code review on the PR, and once fixed up, I think it's ready to merge. Then if you want to publish a 0.7.4 (right?) version, please feel free. Otherwise I can do it.

sandeepnagra commented 4 years ago

@samnissen - I published version 0.7.4