Skip to content

Fix warning on nil comparison #307

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

Merged
merged 1 commit into from
Jan 6, 2025
Merged

Fix warning on nil comparison #307

merged 1 commit into from
Jan 6, 2025

Conversation

ekohl
Copy link
Contributor

@ekohl ekohl commented Dec 10, 2024

This uses the more efficient /regex/.match?(value) syntax that also doesn't raise any warning if id is nil.

This uses the more efficient /regex/.match?(value) syntax that also
doesn't raise any warning if id is nil.
@chris1984 chris1984 merged commit d971ad5 into fog:master Jan 6, 2025
1 of 5 checks passed
@ekohl ekohl deleted the fix-warning branch January 6, 2025 16:25
@ekohl
Copy link
Contributor Author

ekohl commented Jan 6, 2025

@chris1984 are you sure the change didn't cause those test failures?

@chris1984
Copy link
Collaborator

chris1984 commented Jan 6, 2025

Blah, I did not mean to hit merge, was adding that I would start testing it with the quick responses and my browser crashed. Let me revert and test it, if it's not related we can remerge. Acked it based on the code looking fine at a first glance.

@nofaralfasi
Copy link
Collaborator

I believe this change is causing errors in Foreman's test suite.

@ekohl
Copy link
Contributor Author

ekohl commented Apr 17, 2025

I believe this change is causing errors in Foreman's test suite.

Can you elaborate?

@nofaralfasi
Copy link
Collaborator

This change introduces the following error, when running the tests on foreman/test/models/compute_resources/vmware_test.rb:

Error:
Foreman::Model::VmwareTest::#clone_vm#test_0004_passes customspec when user_data is a valid yaml hash:
TypeError: no implicit conversion of Integer into String
    app/models/compute_resources/foreman/model/vmware.rb:605:in `clone_vm'
    test/models/compute_resources/vmware_test.rb:715:in `block (2 levels) in <class:VmwareTest>'

Error:
Foreman::Model::VmwareTest::#clone_vm#test_0003_ignores customspec when user_data is nil:
TypeError: no implicit conversion of Integer into String
    app/models/compute_resources/foreman/model/vmware.rb:605:in `clone_vm'
    test/models/compute_resources/vmware_test.rb:706:in `block (2 levels) in <class:VmwareTest>'

We are encountering this error when client.servers.get is called on an Integer (here).

@ekohl
Copy link
Contributor Author

ekohl commented Apr 17, 2025

That could be related, but previously that also generated a warning I think. A better check is probably is_a?(String) && ....

@ekohl
Copy link
Contributor Author

ekohl commented Apr 17, 2025

Expanding on that: it was a bug before anyway on modern Ruby anyway:

$ pry
[1] pry(main)> 5 =~ /a/
NoMethodError: undefined method `=~' for an instance of Integer (NoMethodError)
from (pry):1:in `__pry__'

IIRC Ruby 3.0 already displayed a deprecation warning and this was trying to address it. I'll submit a PR.

@ekohl
Copy link
Contributor Author

ekohl commented Apr 17, 2025

#310

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants