Add Rake tasks for update, remove, and version. #117

kapoorlakshya · 2019-05-14T05:18:35Z

rake webdrivers:chromedriver:remove  # Removes chromedriver
rake webdrivers:chromedriver:update  # Updates chromedriver
rake webdrivers:geckodriver:remove   # Removes geckodriver
rake webdrivers:geckodriver:update   # Updates geckodriver
rake webdrivers:iedriver:remove      # Removes IEServerDriver
rake webdrivers:iedriver:update      # Updates IEServerDriver

Closes #28.

twalpole · 2019-05-14T13:40:10Z

I think the update tasks probably need to accept version numbers since any code specifying a version number to lock to won’t be loaded when these are run

twalpole · 2019-05-14T13:46:37Z

Additionally the new tasks should not be in the main Rakefile - they should be in a separate file other projects require - See http://mdzhang.com/blog/code/2016/09/10/create-ruby-gem-that-adds-rake-tasks/ - ignore the part about separate files for each task though - that seems unnecessary

titusfortner · 2019-05-14T16:13:22Z

We also want to be able to use caching with update.

I'm thinking we probably want to leverage --cache-time and --required-version parameters with an option parser (http://www.mikeball.info/blog/rake-option-parser/)

The description should be more clear; something like:
"force removal of driver"
and
"remove and download updated driver if necessary"

Info about this should also get added to the README

Finally, I think we want to do Webdrivers.logger.info = 'message' instead of puts

kapoorlakshya · 2019-05-15T06:29:19Z

@twalpole @titusfortner I wasn't sure of the expectation at first, but your comments and links made them very clear. Thanks for that :)

Updated task descriptions:

$ bundle exec rake -T
rake webdrivers:chromedriver:remove  # Force remove chromedriver
rake webdrivers:chromedriver:update  # Remove and download updated chromedriver if necessary
rake webdrivers:geckodriver:remove   # Force remove geckodriver
rake webdrivers:geckodriver:update   # Remove and download updated geckodriver if necessary
rake webdrivers:iedriver:remove      # Force remove IEDriverServer
rake webdrivers:iedriver:update      # Remove and download updated IEDriverServer if necessary

Added options for update task:

-c, --cache-time SECONDS
-v, --required-version VERSION

-t is taken by rake for --trace. Feel free to suggest a better one for --cache-time.

-h, --help option:

$ bundle exec rake webdrivers:chromedriver:update -- -h
Usage: webdrivers::chromedriver::update [options]
    -c, --cache-time SECONDS         (Optional) Number of Seconds to wait between update checks
    -v, --required-version VERSION   (Optional) chromedriver version to download

remove task:

$ bundle exec rake webdrivers:chromedriver:remove
2019-05-14 23:10:29 INFO Webdrivers Removed chromedriver 74.0.3729.6.

$ bundle exec rake webdrivers:chromedriver:remove
2019-05-14 23:10:58 INFO Webdrivers No existing chromedriver to remove.

I would like to DRY up the OptionParser code, but the only way I can think of is wrapping it inside a module (helper method) and passing the driver class name to it for the option descriptions. Do you have any strategies on how to make it cleaner, if needed?

I will update the README once we finalize on the code.

twalpole · 2019-05-15T10:20:16Z

Rake can take multiple tasks on the same command line (rake webdrivers:chromedriver:update spec for instance), so switches don't generally work well. The normal rake way to do it would be either using the built in parameter parsing - https://github.com/ruby/rake/blob/master/doc/rakefile.rdoc#tasks-with-arguments - which would then be called like rake webdriver:chromedriver:update[73.0329.123.11] or via environment variables rake webdriver:chromedriver:update version="73.x.x.x"(ENV['version'] would access the version). Obviously the second setup has the same issue as using the switches like it's currently implemented ( You can't do rake webdriver:chromedriver:update webdriver:geckodriver:update because there would be no way to pass 2 versions)

twalpole · 2019-05-15T21:11:32Z

Additionally please add Railtie support as described in the previous link I sent, so that for users of Rails the rake tasks get automatically loaded.

titusfortner · 2019-05-15T21:42:06Z

@twalpole I haven't tested it, but wouldn't this work as expected?

bundle exec rake webdrivers:chromedriver:update spec -- --cache-time 30 --version 74.0.3729.6

Of note, users will also need to set :cache_time in the code as well so that it doesn't go back online during each individual test run following the rake task. So it might make sense to have this be an environment variable set in the rake task and have our code respect it.

twalpole · 2019-05-15T23:15:12Z

@titusfortner that might, but what if my tests run with chromedriver and geckodriver - then you can't set multiple versions, however

bundle exec rake webdrivers:chromedriver:update[74.0.3729.6] webdrivers:geckodriver:update[0.24.0] spec CACHE_TIME=30

would allow it (and simplifies the tasks by removing the need for OptionParser in favor of using rakes native features).

twalpole · 2019-05-15T23:21:57Z

hmm - of course that then breaks in zsh by default and users need to escape the [ and ] on the command line - 🤷‍♂

kapoorlakshya · 2019-05-16T05:39:33Z

Updated the tasks as per the discussion here and on Slack:

$ bundle exec rake -T
rake webdrivers:chromedriver:remove           # Force remove chromedriver
rake webdrivers:chromedriver:update[version]  # Remove and download updated chromedriver if necessary
rake webdrivers:geckodriver:remove            # Force remove geckodriver
rake webdrivers:geckodriver:update[version]   # Remove and download updated geckodriver if necessary
rake webdrivers:iedriver:remove               # Force remove IEDriverServer
rake webdrivers:iedriver:update[version]      # Remove and download updated IEDriverServer if necessary

The following works now and CACHE_TIME defaults to 86_400:

$ bundle exec rake webdrivers:chromedriver:update[2.46] webdrivers:geckodriver:update[0.24.0] CACHE_TIME=30
2019-05-15 22:36:41 INFO Webdrivers Updated to chromedriver 2.46.628388
2019-05-15 22:36:41 INFO Webdrivers Updated to geckodriver 0.24.0

@twalpole Added Railtie support. Can you please test it when you get a chance?

titusfortner · 2019-05-16T19:23:21Z

Yeah, this looks really good. I still like the OptionParser better as it is more explicit, and then just tell people to create their own rake file if they want to combine them, but whatever, let's do it the default Rake way. :)

Only two changes:

We should support ENV['CACHE_TIME'] in the code as well as the Rake tasks.
I know it works, but we should also have a spec for things working when we set required_version equal to zero since that's what this Rake task is doing

twalpole · 2019-05-16T19:57:27Z

Tested out the railties - seems to work fine

RAILS_ENV=test rake -T
Running via Spring preloader in process 35379
rake about                                    # List versions of all Rails frameworks and the environment
rake active_storage:install                   # Copy over the migration needed to the application
rake app:template                             # Applies the template supplied by LOCATION=(/path/to/template) or URL
...
rake tmp:create                               # Creates tmp directories for cache, sockets, and pids
rake webdrivers:chromedriver:remove           # Force remove chromedriver
rake webdrivers:chromedriver:update[version]  # Remove and download updated chromedriver if necessary
rake webdrivers:geckodriver:remove            # Force remove geckodriver
rake webdrivers:geckodriver:update[version]   # Remove and download updated geckodriver if necessary
rake webdrivers:iedriver:remove               # Force remove IEDriverServer
rake webdrivers:iedriver:update[version]      # Remove and download updated IEDriverServer if necessary
rake yarn:install                             # Install all JavaScript dependencies as specified via Yarn

One thing I did think about is the install dir - I think we probably want a way to specify that for these tasks too - thoughts?

kapoorlakshya · 2019-05-16T20:15:32Z

@titusfortner I'll add support for ENV['CACHE_TIME'] in the core code. And I am okay with adding a spec for required_version = 0 or we can set it to Gem::Version('') as PR #108 added logic to handle it.

@twalpole Sounds reasonable. How about bundle exec rake webdrivers:chromedriver:update[2.46] webdrivers:geckodriver:update[0.24.0] CACHE_TIME=30 INSTALL_DIR='my_dir'?

Additionally, I would like to rename CACHE_TIME to WD_CACHE_TIME (and WD_INSTALL_DIR) so it's a bit more specific to this gem via the "WD" notation. That way we can set it at the global level and avoid possible conflicts with any other INSTALL_DIR or caching related ENV variables from other tools/user's project.

kapoorlakshya · 2019-05-17T19:27:21Z

Note to myself:

Implement support for ENV['WD_CACHE_TIME'] and ENV['WD_INSTALL_DIR'] in the core code.
Update rake tasks to use WD_CACHE_TIME
Add specs to test required_version as nil and 0. Latest version should be downloaded.
Implement support for WD_INSTALL_DIR in update rake task
Add rake task webdrivers:chromedriver:version to print current_version
Update README.

kapoorlakshya · 2019-05-19T00:47:07Z

Here's the updated list:

$ bundle exec rake -T
rake webdrivers:chromedriver:remove           # Force remove chromedriver
rake webdrivers:chromedriver:update[version]  # Remove and download updated chromedriver if necessary
rake webdrivers:chromedriver:version          # Print current chromedriver version
rake webdrivers:geckodriver:remove            # Force remove geckodriver
rake webdrivers:geckodriver:update[version]   # Remove and download updated geckodriver if necessary
rake webdrivers:geckodriver:version           # Print current geckodriver version
rake webdrivers:iedriver:remove               # Force remove IEDriverServer
rake webdrivers:iedriver:update[version]      # Remove and download updated IEDriverServer if necessary
rake webdrivers:iedriver:version              # Print current IEDriverServer version

:version task:

$ bundle exec rake webdrivers:chromedriver:version
2019-05-18 17:45:59 INFO Webdrivers chromedriver 74.0.3729.6

titusfortner

LGTM. Couple notes, then I'm cool with merging. Next release will be a beta to make sure it works for users, so they can let us know if there are issues or use cases we overlooked... :)

titusfortner · 2019-05-20T19:28:43Z

lib/webdrivers/system.rb

@@ -26,7 +26,7 @@ def delete(file)
      end

      def install_dir
-        Webdrivers.install_dir || File.expand_path(File.join(ENV['HOME'], '.webdrivers'))
+        Webdrivers.install_dir


is this different from just using the attr_accessor?

The accessor (reader method) was only returning the user define path or nil. I attempted to move all the logic in System.install_dir and have Webdrivers.install_dir reference it, but then that created a circular reference as the first check in System is for Webdrivers.install_dir, which itself was referencing System.install_dir to get the default path.

lib/webdrivers/tasks/chromedriver.rake

lib/webdrivers/tasks/geckodriver.rake

lib/webdrivers/tasks/iedriver.rake

titusfortner · 2019-05-20T19:41:48Z

Oh, this also needs a README update to discuss ENV specifics & Available tasks

kapoorlakshya · 2019-05-21T02:31:44Z

Made requested changes and updated README.

kapoorlakshya force-pushed the add_rake_tasks branch 2 times, most recently from d6d9643 to c10c6ba Compare May 15, 2019 06:32

kapoorlakshya force-pushed the add_rake_tasks branch from c10c6ba to 626b60c Compare May 16, 2019 05:39

kapoorlakshya force-pushed the add_rake_tasks branch from 626b60c to a43200f Compare May 16, 2019 16:31

kapoorlakshya force-pushed the add_rake_tasks branch 6 times, most recently from dbeb231 to ddec5af Compare May 19, 2019 00:41

kapoorlakshya changed the title ~~Add Rake tasks for update & remove.~~ Add Rake tasks for update, remove, and version. May 19, 2019

kapoorlakshya force-pushed the add_rake_tasks branch 2 times, most recently from 17e02d1 to fc075a3 Compare May 19, 2019 00:55

titusfortner approved these changes May 20, 2019

View reviewed changes

Add rake tasks for update, remove, and version.

5c12329

kapoorlakshya force-pushed the add_rake_tasks branch from f3b2b9d to 5c12329 Compare May 21, 2019 02:31

titusfortner merged commit 050e8ce into titusfortner:master May 21, 2019

This was referenced May 21, 2019

Add executables for chromedriver and geckodriver #120

Closed

parallelization issues #77

Closed

kapoorlakshya deleted the add_rake_tasks branch May 21, 2019 16:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Rake tasks for update, remove, and version. #117

Add Rake tasks for update, remove, and version. #117

kapoorlakshya commented May 14, 2019

twalpole commented May 14, 2019

twalpole commented May 14, 2019 •

edited

Loading

titusfortner commented May 14, 2019

kapoorlakshya commented May 15, 2019

twalpole commented May 15, 2019 •

edited

Loading

twalpole commented May 15, 2019

titusfortner commented May 15, 2019

twalpole commented May 15, 2019 •

edited

Loading

twalpole commented May 15, 2019

kapoorlakshya commented May 16, 2019 •

edited

Loading

titusfortner commented May 16, 2019

twalpole commented May 16, 2019 •

edited

Loading

kapoorlakshya commented May 16, 2019

kapoorlakshya commented May 17, 2019 •

edited

Loading

kapoorlakshya commented May 19, 2019

titusfortner left a comment

titusfortner May 20, 2019

kapoorlakshya May 21, 2019

titusfortner commented May 20, 2019

kapoorlakshya commented May 21, 2019

Add Rake tasks for update, remove, and version. #117

Add Rake tasks for update, remove, and version. #117

Conversation

kapoorlakshya commented May 14, 2019

twalpole commented May 14, 2019

twalpole commented May 14, 2019 • edited Loading

titusfortner commented May 14, 2019

kapoorlakshya commented May 15, 2019

twalpole commented May 15, 2019 • edited Loading

twalpole commented May 15, 2019

titusfortner commented May 15, 2019

twalpole commented May 15, 2019 • edited Loading

twalpole commented May 15, 2019

kapoorlakshya commented May 16, 2019 • edited Loading

titusfortner commented May 16, 2019

twalpole commented May 16, 2019 • edited Loading

kapoorlakshya commented May 16, 2019

kapoorlakshya commented May 17, 2019 • edited Loading

kapoorlakshya commented May 19, 2019

titusfortner left a comment

Choose a reason for hiding this comment

titusfortner May 20, 2019

Choose a reason for hiding this comment

kapoorlakshya May 21, 2019

Choose a reason for hiding this comment

titusfortner commented May 20, 2019

kapoorlakshya commented May 21, 2019

twalpole commented May 14, 2019 •

edited

Loading

twalpole commented May 15, 2019 •

edited

Loading

twalpole commented May 15, 2019 •

edited

Loading

kapoorlakshya commented May 16, 2019 •

edited

Loading

twalpole commented May 16, 2019 •

edited

Loading

kapoorlakshya commented May 17, 2019 •

edited

Loading