titusfortner / webdrivers

Keep your Selenium WebDrivers updated automatically
MIT License
593 stars 111 forks source link

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

Closed kapoorlakshya closed 5 years ago

kapoorlakshya commented 5 years ago
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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

@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 commented 5 years ago

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

kapoorlakshya commented 5 years ago

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 commented 5 years ago

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:

  1. We should support ENV['CACHE_TIME'] in the code as well as the Rake tasks.
  2. 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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

Note to myself:

kapoorlakshya commented 5 years ago

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 commented 5 years ago

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

kapoorlakshya commented 5 years ago

Made requested changes and updated README.