Unlock minitest version of google-cloud-storage

[yahonda]

This pull request bumps the minitest version of google-cloud-storage because #4884 finds newer version of minitest is necessary to support Ruby 2.8 keyword arguments.

Summary of changes:

• Bump minitest of google-cloud-storage version to the latest
• https://github.com/rubocop-hq/rubocop-minitest supports auto-corrections for minitest global expectations warnings
• Most of (kind of) false positives are corrected and restored manually.

Follow up #4884 Related to #4110

[quartzmo]

@yahonda Thank you for this pull request! I will look at it today.

[quartzmo]

Please see current discussion of minitest style on #4110. All opinions welcome.

[yahonda]

Thanks for the update. I'm following discussions on #4110.

[dazuma]

This LGTM, and I think we should take it as-is, to enable Ruby 2.7. (We do want to switch to asserts at some point, but that's a much lower priority and can be dealt with later.)

[quartzmo]

@yahonda

1. The acceptance directory also contains many minitest assertions which need to be updated. To run the acceptance (integration) tests, set your GCP credentials as described in AUTHENTICATION.md#environment-variables and run bundle exec rake acceptance.

2. I pulled this branch and ran bundle exec rake test locally (with Ruby 2.6.5p114 and minitest 5.14.0). I get these deprecation warnings:

*********************************
*** google-cloud-storage test ***
*********************************

Run options: --seed 41721

# Running:

.............................................................................DEPRECATED: global use of must_be_kind_of from /Users/quartzmo/.rbenv/versions/2.6.5/lib/ruby/2.6.0/delegate.rb:83. Use _(obj).must_be_kind_of instead. This will fail in Minitest 6.
........................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................DEPRECATED: global use of must_be_kind_of from /Users/quartzmo/.rbenv/versions/2.6.5/lib/ruby/2.6.0/delegate.rb:83. Use _(obj).must_be_kind_of instead. This will fail in Minitest 6.
..DEPRECATED: global use of must_be_kind_of from /Users/quartzmo/.rbenv/versions/2.6.5/lib/ruby/2.6.0/delegate.rb:83. Use _(obj).must_be_kind_of instead. This will fail in Minitest 6.
....................DEPRECATED: global use of must_be_kind_of from /Users/quartzmo/.rbenv/versions/2.6.5/lib/ruby/2.6.0/delegate.rb:83. Use _(obj).must_be_kind_of instead. This will fail in Minitest 6.
..............................................................................................................................................................................DEPRECATED: global use of must_be_kind_of from /Users/quartzmo/.rbenv/versions/2.6.5/lib/ruby/2.6.0/delegate.rb:83. Use _(obj).must_be_kind_of instead. This will fail in Minitest 6.
....DEPRECATED: global use of must_be_kind_of from /Users/quartzmo/.rbenv/versions/2.6.5/lib/ruby/2.6.0/delegate.rb:83. Use _(obj).must_be_kind_of instead. This will fail in Minitest 6.
...........DEPRECATED: global use of must_be_kind_of from /Users/quartzmo/.rbenv/versions/2.6.5/lib/ruby/2.6.0/delegate.rb:83. Use _(obj).must_be_kind_of instead. This will fail in Minitest 6.
..DEPRECATED: global use of must_be_kind_of from /Users/quartzmo/.rbenv/versions/2.6.5/lib/ruby/2.6.0/delegate.rb:83. Use _(obj).must_be_kind_of instead. This will fail in Minitest 6.
...............

Finished in 0.911296s, 957.9763 runs/s, 2692.8682 assertions/s.

873 runs, 2454 assertions, 0 failures, 0 errors, 0 skips

[quartzmo]

@yahonda Please rebase on master branch before updating this PR, since I merged #4866 which contains changes to existing tests.

[quartzmo]

@yahonda I can take over this PR if you're busy, just let me know.

[yahonda]

Thanks for the friendly reminder. I'll take care of this pull request. I may ask some questions about GCP credentials if necessary.

[yahonda]

I have performed git rebase from the master branch.

Here are answers for questions:

1. The acceptance directory also contains many minitest assertions which need to be updated. To run the acceptance (integration) tests, set your GCP credentials as described in AUTHENTICATION.md#environment-variables and run bundle exec rake acceptance.

I have configured GCP credentials and tried to let rubocop-minitest auto-correct these warnings. It looks like rubocop-minitest does not execute auto-correct files under acceptance. I have opened rubocop/rubocop-minitest#73 to get some help.

2. I pulled this branch and ran bundle exec rake test locally (with Ruby 2.6.5p114 and minitest 5.14.0). I get these deprecation warnings:

Yes, I was aware of them and I should have explained it in advance. rubocop-minitest auto-correct causes some test failures explained the commit linked below.

f290e7b

I think rubocop-minitest is not able to perform auto-correct correctly because it is a static code analyzer then it should not know the Tempfile delegates to File. I still have no idea how to fix them.

[quartzmo]

CI build is failing at bundle exec rake rubocop:

...
acceptance/storage_helper.rb:154:30: C: Style/GlobalVars: Do not introduce global variables.
    clean_up_storage_buckets $storage_2, $bucket_names_2, user_project: true
                             ^^^^^^^^^^
acceptance/storage_helper.rb:154:42: C: Style/GlobalVars: Do not introduce global variables.
    clean_up_storage_buckets $storage_2, $bucket_names_2, user_project: true
                                         ^^^^^^^^^^^^^^^

49 files inspected, 293 offenses detected
RuboCop failed!

The acceptance directory should not be included in Rubocop coverage for this package.

[yahonda]

Right. Reverted 7f279bf to exclude the acceptance directory.

[yahonda]

Here is a sample script to explain why I reverted some changes by rubocop-minitest and why it shows warnings now.

Just updating the test file as suggested by DEPRECATED warning, it gets failure. Changing from .must_be_kind_of File to .must_be_kind_of Tempfile addresses these warnings and failures but not still 100% if it satisfies the test purpose yet.

• rep.rb

# frozen_string_literal: true

require 'bundler/inline'
require 'tempfile'

gemfile(true) do
  source 'https://rubygems.org'
  git_source(:github) { |repo| "https://github.com/#{repo}.git" }
  gem 'minitest'
end

require 'minitest/autorun'
describe Tempfile do
  it 'temp file must be kind of file' do
    # Success with warnings
    t = Tempfile.new
    t.must_be_kind_of File
  end

  it 'underscore temp file must be kind of file' do
    # Failure
    t = Tempfile.new
    _(t).must_be_kind_of File
  end

  it 'underscore temp file must be kind of tempfile' do
    # Success without warnings
    t = Tempfile.new
    _(t).must_be_kind_of Tempfile
  end
end

• Result

% ruby rep.rb
Fetching gem metadata from https://rubygems.org/..................
Resolving dependencies...
Using bundler 2.1.4
Using minitest 5.14.0
Run options: --seed 34409

# Running:

DEPRECATED: global use of must_be_kind_of from /Users/yahonda/.rbenv/versions/2.7.1/lib/ruby/2.7.0/delegate.rb:83. Use _(obj).must_be_kind_of instead. This will fail in Minitest 6.
.F.

Finished in 0.001370s, 2189.7810 runs/s, 2189.7810 assertions/s.

  1) Failure:
Tempfile#test_0002_underscore temp file must be kind of file [rep.rb:23]:
Expected #<Tempfile:/var/folders/4d/1hfsdkkj2772brj5d70vtbm40000gn/T/20200412-1792-1ygtgv6> to be a kind of File, not Tempfile.

3 runs, 3 assertions, 1 failures, 0 errors, 0 skips
%

[yahonda]

I think we can replace .must_be_kind_of File with .must_be_kind_of Tempfile because of these reasons:

• must_be_kind_of and assert_kind_of are synonym and assert_kind_of
• assert_kind_of fails unless the object is a kind of class
• A instance of Tempfile is not kind of a File

• Minitest references
  http://docs.seattlerb.org/minitest/Minitest/Expectations.html#method-i-must_be_kind_of
  http://docs.seattlerb.org/minitest/Minitest/Assertions.html#method-i-assert_kind_of

assert_kind_of(cls, obj, msg = nil)
Fails unless obj is a kind of cls.

• Simple script

require 'tempfile'
t = Tempfile.new
p t.is_a? Tempfile # true
p t.is_a? File # false
p t.kind_of? Tempfile # true
p t.kind_of? File # false
p t.class.ancestors

• Result:

true
false
true
false
[Tempfile, #<Class:0x00007fd205097e08>, Delegator, #<Module:0x00007fd20401ad00>, BasicObject]

[yahonda]

I have addressed all deprecation warnings under the acceptance directory. It is ready to review.

[yahonda]

Reverted two commits because RuboCop Minitest 0.9.0 has been released. https://github.com/rubocop-hq/rubocop-minitest/releases/tag/v0.9.0

[quartzmo]

I pulled the latest and ran rake ci and rake acceptance, both successful with no warnings.

@yahonda Can we remove the rubocop-minitest dependency now that the changes have been made?

[quartzmo]

@yahonda Thank you so much for this pull request and the effort you put in. Much appreciated!

[yahonda]

@quartzmo Thanks for the kind response. I'm happy to unlock minitest versions of other modules at google-cloud-ruby repository. Let me know if you have any concerns.