-
Notifications
You must be signed in to change notification settings - Fork 991
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes #36849 - Run GHA on Ruby 3.0 #9989
Conversation
fde7cd8
to
2a7cda3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a quick look at this and it appears the approach is to make the constructor's signature match the base class. That makes sense to me.
As for STI, I'm not sure we can get rid of it. I'd like to. At this moment I think we only have 2 implementations, so be sure to check out what Discovery does.
2a7cda3
to
c6dd66a
Compare
c6dd66a
to
d49fd7e
Compare
@@ -1,5 +1,5 @@ | |||
{ | |||
"postgresql": ["12"], | |||
"ruby": ["2.7"], | |||
"ruby": ["2.7", "3.0"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we separate this out to its own commit? I'd like to merge all the preparation work before we pull the trigger.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some deprecation warnings when running tests:
2024-01-23T18:54:00.9931233Z Mocha deprecation warning at /home/runner/work/foreman/foreman/app/services/proxy_api/dhcp.rb:36:in `unused_ip': Expectation defined at /home/runner/work/foreman/foreman/test/unit/proxy_api/dhcp_test.rb:99:in `block in <class:ProxyApiDhcpTest>' expected positional hash ({:query => "mac=00:11:22:33:44:55"}), but received keyword arguments (:query => "mac=00:11:22:33:44:55"). These will stop matching when strict keyword argument matching is enabled. See the documentation for Mocha::Configuration#strict_keyword_argument_matching=.
2024-01-23T18:54:01.0281310Z Mocha deprecation warning at /home/runner/work/foreman/foreman/app/services/proxy_api/dhcp.rb:36:in `unused_ip': Expectation defined at /home/runner/work/foreman/foreman/test/unit/proxy_api/dhcp_test.rb:74:in `block in <class:ProxyApiDhcpTest>' expected positional hash ({:query => ""}), but received keyword arguments (:query => ""). These will stop matching when strict keyword argument matching is enabled. See the documentation for Mocha::Configuration#strict_keyword_argument_matching=.
2024-01-23T18:54:01.0407752Z Mocha deprecation warning at /home/runner/work/foreman/foreman/app/services/proxy_api/dhcp.rb:36:in `unused_ip': Expectation defined at /home/runner/work/foreman/foreman/test/unit/proxy_api/dhcp_test.rb:90:in `block in <class:ProxyApiDhcpTest>' expected positional hash ({:query => "from=192.168.0.50&to=192.168.0.150"}), but received keyword arguments (:query => "from=192.168.0.50&to=192.168.0.150"). These will stop matching when strict keyword argument matching is enabled. See the documentation for Mocha::Configuration#strict_keyword_argument_matching=.
2024-01-23T18:54:13.7290895Z /home/runner/work/foreman/foreman/vendor/bundle/ruby/2.7.0/gems/fog-vsphere-3.6.3/lib/fog/vsphere/compute.rb:344: warning: deprecated Object#=~ is called on Integer; it always returns nil
2024-01-23T18:54:13.7458370Z /home/runner/work/foreman/foreman/vendor/bundle/ruby/2.7.0/gems/fog-vsphere-3.6.3/lib/fog/vsphere/compute.rb:344: warning: deprecated Object#=~ is called on Integer; it always returns nil
2024-01-23T18:54:22.8953091Z Mocha deprecation warning at /home/runner/work/foreman/foreman/app/models/host/managed.rb:715:in `ipmi_boot': Expectation defined at /home/runner/work/foreman/foreman/test/models/host_test.rb:3245:in `block (2 levels) in <class:HostTest>' expected keyword arguments (:function => "bootdevice", :device => "bios"), but received positional hash ({:function => "bootdevice", :device => "bios"}). These will stop matching when strict keyword argument matching is enabled. See the documentation for Mocha::Configuration#strict_keyword_argument_matching=.
The fog-vsphere warnings will break with Ruby 3.2. The other ones look like some things we should fix in our code and then enable strict_keyword_argument_matching
in mocha.
d49fd7e
to
0aab057
Compare
Can we also enable strict keyword argument matching (https://mocha.jamesmead.org/Mocha/Configuration.html#strict_keyword_argument_matching=-instance_method)? Perhaps make that the whole second commit? |
ecb8a73
to
047dceb
Compare
Done. Not sure though if we need that. As there was stated, this will be |
047dceb
to
b17c9c8
Compare
That way we don't accidentally regress now that we fixed the offenders until it's |
b17c9c8
to
eefdccd
Compare
Indeed. If it was |
Some Katello test failures are related to strict keyword argument matching I guess, but otherwise 🍏 for Foreman. |
I've love to leverage what you did in #9989 (comment) and see how other repositories behave. |
I'm preparing more PRs regarding what I found in Katello, I'll paste them here once they are ready. The plan to go over my locally checkedout other plugins as well. |
@ekohl, there is another, more general and detailed approach trying to fix the same issue. I hope the code and the comments will help to understand the idea of what's going wrong.
Unfortunately, that's the last thing I could find. The third and (that's only my opinion) the last option would be to completely get rid of that weird
app/models/host.rb
module, which (per my understanding) simply tries to simulate an abstract class.UPD: TIL that rubocop can apparently yell that the indentation of a comment can be wrong and that's unacceptable 🤦
Alternative to #9871.