Fix missing reason constants #13919
Fix missing reason constants #13919
Conversation
* remove any_instance_of since we have an instance of a server! 🎉 * use lets, remove some ivars https://bugzilla.redhat.com/show_bug.cgi?id=1395736
Due to module inclusion spaghetti, it's easier and less confusing to reference the Reason constants consistently in the MiqServer class, which is the ultimate destination for all of these modules. Fixes ManageIQ#13901 introduced in ManageIQ#13805 https://bugzilla.redhat.com/show_bug.cgi?id=1395736
|
Checked commits jrafanie/manageiq@6636853~...3ea55fa with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
There was a problem hiding this comment.
The monitor part looks good and works - tested here.
@chrisarcand merge?
| @worker1 = FactoryGirl.create(:miq_worker, :miq_server_id => @miq_server.id, :memory_usage => 2.gigabytes, :pid => 42) | ||
| allow_any_instance_of(MiqServer).to receive(:get_time_threshold).and_return(2.minutes) | ||
| allow_any_instance_of(MiqServer).to receive(:get_memory_threshold).and_return(500.megabytes) | ||
| allow_any_instance_of(MiqServer).to receive(:get_restart_interval).and_return(0.hours) |
There was a problem hiding this comment.
Not even a 👏 from @chrisarcand. 😢
I was so excited to be able to remove allow_any_instance_of. 😉
There was a problem hiding this comment.
Haha! Funny story is that I saw it, screamed with joy, and hit merge as I was headed out the door with no time to comment.
WOOOOO!!!!! 😱 ❤️ 😍 😘 💋 🎉 🎊 👏 💯 🏅
There was a problem hiding this comment.
Haha! Nice emoji variety. Funny story: I looked at those tests and couldn't understand why it was using allow_any_instance_of. I was shocked and overly excited when the tests still passed using the server instance we already had!
There was a problem hiding this comment.
hit merge as I was headed out the door with no time to comment.
Drive-by merging...
…ntants Fix missing reason contants (cherry picked from commit 89dfba8) https://bugzilla.redhat.com/show_bug.cgi?id=1395736
…ntants Fix missing reason contants (cherry picked from commit 89dfba8) https://bugzilla.redhat.com/show_bug.cgi?id=1395736
|
Backported to Euwe via #13949 |
|
Backported to Darga via #13950 |
The first commit is just a refactoring/cleanup so I can add the fix and it's test in the second commit.
Fixes #13901 introduced in #13805
Fix Reason NameError using constants included in MiqServer
Due to module inclusion spaghetti, it's easier and less confusing
to reference the Reason constants consistently in the MiqServer class,
which is the ultimate destination for all of these modules.
https://bugzilla.redhat.com/show_bug.cgi?id=1395736
@durandom Please review