(PUP-10537) Resolve Ruby 2.7 Warnings #8179
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This is blocked on #8175 |
|
CLA signed by all contributors. |
melissa
force-pushed
the
maint/master/ruby-2.7
branch
4 times, most recently
from
ef9435c
to
ac71e39
Compare
|
I'm not going to be able to fix all the warnings. rspec-mocks currently is raising a warning with Ruby 2.7 when we use |
melissa
force-pushed
the
maint/master/ruby-2.7
branch
from
597a473
to
8f2dbc8
Compare
|
I was just going to leave the commit messages as they are, unless you think I should squash them down by type. |
joshcooper
reviewed
May 28, 2020
joshcooper
reviewed
May 28, 2020
joshcooper
reviewed
May 28, 2020
joshcooper
reviewed
May 28, 2020
joshcooper
reviewed
May 28, 2020
joshcooper
reviewed
May 28, 2020
joshcooper
reviewed
May 28, 2020
joshcooper
reviewed
May 28, 2020
joshcooper
reviewed
May 28, 2020
joshcooper
reviewed
May 28, 2020
joshcooper
reviewed
May 28, 2020
joshcooper
reviewed
May 28, 2020
melissa
force-pushed
the
maint/master/ruby-2.7
branch
from
7f37549
to
c83742c
Compare
melissa
force-pushed
the
maint/master/ruby-2.7
branch
from
c83742c
to
0407db6
Compare
|
I forgot I can't enable Ruby 2.7 testing with Travis yet because of the |
melissa
force-pushed
the
maint/master/ruby-2.7
branch
from
0407db6
to
f2d1657
Compare
melissa
commented
Jun 1, 2020
melissa
force-pushed
the
maint/master/ruby-2.7
branch
2 times, most recently
from
4ad229d
to
dbd750e
Compare
melissa
commented
Jun 3, 2020
| super(path, data.map { |k, v| [k.to_sym, v] }.to_h) | ||
| links = data.fetch('links', nil) || data.fetch(:links, nil) | ||
| relative_path = data.fetch('relative_path', nil) || data.fetch(:relative_path, nil) | ||
| source = @source || data.fetch(:source, nil) |
There was a problem hiding this comment.
This isn't the cleanest, but it works...
There was a problem hiding this comment.
ah 👍 It's interesting how making this explicit shows how confusing it is for callers, to pass a string for owner, but symbol or string for links).
There was a problem hiding this comment.
yeah... I had it as strings originally, but the tests failed. So, symbols it is. I'm not confident that owner is necessarily a string, but it's working as is, so...
`warning: deprecated Object#=~ is called on Array; it always returns nil` In order to fix this warning, ensure that `place` is a string prior to checking for a match
`warning: $SAFE will become a normal global variable in Ruby 3.0` `$SAFE` and `taint` are deprecated concepts. See: https://rubyreferences.github.io/rubychanges/2.7.html#safe-and-taint-concepts-are-deprecated-in-general This is largely based on changes in ruby/ruby@c5c0546 This code was originally introduced by puppetlabs@19cf4be, which was based on the ruby code. It makes sense to follow their logic on how we extract `$SAFE` from our code.
`warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call` In preparation for Ruby 3, Ruby 2.7 has added warnings around how keyword and positional arguments are used. Details can be found at: https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/
`warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call` In preparation for Ruby 3, Ruby 2.7 has added warnings around how keyword and positional arguments are used. Details can be found at: https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/ This commit is just covering the changes needed for ssl_provider_spec.rb, though it might be a better idea to merge these commits.
`warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call`
`warning: deprecated Object#=~ is called on Puppet::Type::File; it always returns nil`
Rather than polluting the run output, we should warn if a LoadError
occurs with standard warning protocol in Puppet. This allows for a
more standardized experience with how users are exposed to these
warnings. It also means that we can clean up spec output, which is nice.
The following is rspec output prior to this change. This commit removed
the multiple warnings `Could not load confine test 'osfamily':
LoadError` from popping up in the output.
```
➜ bundle exec rspec --format d --force-color spec/unit/confine_spec.rb:77
Run options:
include {:locations=>{"./spec/unit/confine_spec.rb"=>[77]}}
exclude {:broken=>true, :benchmark=>true}
Puppet::Confine
when requiring
Could not load confine test 'osfamily': LoadError
Could not load confine test 'osfamily': LoadError
does not cache failed requires when always_retry_plugins is true
Could not load confine test 'osfamily': LoadError
caches failed requires when always_retry_plugins is false
```
`warning: deprecated Object#=~ is called on Puppet::Type::File; it always returns nil`
`warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call`
`warning: deprecated Object#=~ is called on Float; it always returns nil`
Both Fixnum and Bignum were unified into Integer in Ruby 2.4. This commit removes the references to them so we do not emit the deprecation warnings when running tests. https://bugs.ruby-lang.org/issues/12005
It appears that in Ruby 2.7 the error message that is raised for
SyntaxErrors is slightly different. This commit updates the regex to
catch the error in 2.7 while maintaining backwards compatability with
earlier ruby versions.
```
1) loaders when a 3x load takes place a function with syntax error has helpful error message
Failure/Error:
expect {
loader.load_typed(typed_name(:function, 'func_with_syntax_error'))
}.to raise_error(/syntax error, unexpected (keyword_|`)?end/)
expected /syntax error, unexpected (keyword_)?end/, got #<SyntaxError: /Users/melissa/workdir/puppet/spec/fixtures/unit/pops/loaders/loaders/mix_4x_and_3x_fu...ib/puppet/parser/functions/func_with_syntax_error.rb:8: syntax error, unexpected `end'
end
^~~
> with backtrace:
# ./lib/puppet/pops/loader/ruby_legacy_function_instantiator.rb:42:in `eval'
# ./lib/puppet/pops/loader/ruby_legacy_function_instantiator.rb:42:in `create'
# ./lib/puppet/pops/loader/module_loaders.rb:284:in `instantiate'
# ./lib/puppet/pops/loader/module_loaders.rb:258:in `find'
# ./lib/puppet/pops/loader/base_loader.rb:163:in `internal_load'
# ./lib/puppet/pops/loader/base_loader.rb:42:in `load_typed'
# ./lib/puppet/pops/loader/dependency_loader.rb:50:in `block in find'
# ./lib/puppet/pops/loader/dependency_loader.rb:48:in `each'
# ./lib/puppet/pops/loader/dependency_loader.rb:48:in `reduce'
# ./lib/puppet/pops/loader/dependency_loader.rb:48:in `find'
# ./lib/puppet/pops/loader/base_loader.rb:163:in `internal_load'
# ./lib/puppet/pops/loader/base_loader.rb:42:in `load_typed'
# ./spec/unit/pops/loaders/loaders_spec.rb:464:in `block (4 levels) in <top (required)>'
# ./spec/unit/pops/loaders/loaders_spec.rb:463:in `block (3 levels) in <top (required)>'
# ./spec/spec_helper.rb:180:in `block (2 levels) in <top (required)>'
# ./spec/unit/pops/loaders/loaders_spec.rb:463:in `block (3 levels) in <top (required)>'
# ./spec/spec_helper.rb:180:in `block (2 levels) in <top (required)>'
Finished in 3 minutes 20.5 seconds (files took 1.38 seconds to load)
1 example, 1 failure
```
melissa
force-pushed
the
maint/master/ruby-2.7
branch
2 times, most recently
from
1af644e
to
ab59036
Compare
|
Trying to enable Ruby 2.7 testing on windows just produced errors |
|
Also, the way we run out tests, with |
joshcooper
approved these changes
Jun 4, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.