NREL / OpenStudio-measure-tester-gem

Other
3 stars 0 forks source link

Update Rubocop from 0.54.0 #56

Closed tijcolem closed 3 years ago

tijcolem commented 3 years ago

Rubocop 0.54.0 has runtime checks to determine ruby version, which you can see here.

However, the only known environment causing this runtime error and preventing rubocop usage is with rbenv (see below)

Platform Ruby Works
Windows x64 RubyInstaller_2.7.2_x64 Yes
Ubuntu 18.04 Ruby 2.7.2 compiled Yes
Ubuntu 18.04 Ruby 2.7.0 apt-get package Yes
Mac 10.15 rvm Ruby 2.7.2 Yes
Mac 10.15 rbenv Ruby 2.7.2 No

To reproduce, clone any of the measure repository (e.g. https://github.com/NREL/openstudio-common-measures-gem) and run a rake task which runs rubocop as part of the process.

Here's an example

export RUBYLIB=/path_to_openstudio_3.2.0-rc1/Ruby
git clone https://github.com/NREL/openstudio-common-measures-gem
cd openstudio-common-measures-gem
ruby --version #  ruby v2.7.2
bundle install 
bundle exec rake openstudio:update_measures
bundle exec rake openstudio:test_with_openstudio

Error message:

========================= Starting Run for Rubocop ================================
Current directory is /Users/nlong/working/openstudio/openstudio-model-articulation-gem
Test results will be stored in: /Users/nlong/working/openstudio/openstudio-model-articulation-gem/test_results
rake aborted!
RuboCop::ValidationError: Unknown Ruby version 2.7 found in `.ruby-version`.
Supported versions: 2.1, 2.2, 2.3, 2.4, 2.5

Using the same rbenv ruby you can also trigger that error by running it directly with the cli.

git clone https://github.com/NREL/openstudio-common-measures-gem
cd openstudio-common-measures-gem
openstudio --version # 3.2.0-rc1+2249bb4700
openstudio --verbose measure -r lib/measures/

I believe the reason is rbenv generates a file .ruby-version which contains the info necessary to determine the running ruby version by rubocop, whereas other rubies on other systems do not produce this file which allows rubocop to run (presumably it's using the default value).

The fix I think is rather simple since newer versions => 0.83.0 of rubocop no longer have a native dependency on jaro_winkler which was problematic for OpenStudio SDK. We can update to the latest, which at this time is 1.13.0.

The new version does impose some changes though. It will requires updated style/cop rule as a few rules have changed. Most repo that use rubocop are deriving from: http://s3.amazonaws.com/openstudio-resources/styles/rubocop_v3.yml. These are the offending rules that are no longer compatible.

# NL 2020-05-03: Allow catching StandardError
 Lint/HandleExceptions:
   Enabled: false

 # NL 2020-05-03: match -> match? or =~ -> match? breaks certain OpenStudio measures.
 Performance/RegexpMatch:
   Enabled: false

 # NL 2020-05-03: Allow gsub (instead of the preferred tr)
 Performance/StringReplacement:
   Enabled: false

Lint/HandleExceptions. just needs a name change but the Performance rules are now moved to their own gem which will need to be added if these are to be included. See: https://github.com/rubocop/rubocop-performance

Unfortunately, all git repos that have this https://github.com/NREL/OpenStudio-measure-tester-gem/blob/develop/.rubocop.yml#L5 defined will need to be updated to ref the updated version.

tijcolem commented 3 years ago

@nllong @kflemin Do we want to try and update this in order for OpenStudio 3.2.1? It will require gems that use measure tests to be updated and released.

kflemin commented 3 years ago

I think we should! @nllong?

nllong commented 3 years ago

Yes we definitely should try to make this happen for the next release! What do you need from me @tijcolem ?

tijcolem commented 3 years ago

@nllong I can initiate the process. It'll a require a new version of the rubocop style rules we have defined in s3. I'll get those updated. Once things are working here in measure tester, then we'll create have to create versions for all the dependent gems. I'm wondering if it makes sense to have all the gems that use OpenStudio-measure-tester inherit the s3 link defined here. https://github.com/NREL/OpenStudio-measure-tester-gem/blob/develop/.rubocop.yml

nllong commented 3 years ago

Great, thanks. Yeah, I think it makes sense to have the gems inherit that link (although a new v4.yml link). I think I tried to have it inherit 'magically' but that didn't work thus I had to add it to every single dependent gem. If you can find a way to make this work, then all the better! 🔥