[GAPRINDASHVILI] Compare decimal columns correctly in batch saver #17214
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
agrare
approved these changes
Mar 28, 2018
|
This pull request is not mergeable. Please rebase and repush. |
…zable_keys"" This reverts commit 9eab6ed, again merging pull request ManageIQ#17020 from cben/deserializable_keys Compare decimal columns correctly in batch saver (cherry picked from commit d3f7348) https://bugzilla.redhat.com/show_bug.cgi?id=1559544
cben
force-pushed
the
GAPRINDASHVILI-deserializable_keys
branch
from
df46123 to
2d79f26
Compare
|
Checked commits cben/manageiq@b45b8de~...2d79f26 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 spec/models/manager_refresh/save_inventory/saver_strategies_spec.rb
|
|
miq-bot Y U NO pay attention? |
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.
Backport (again) #17020, the only difference being @Ladas's
.to_stest fix #17194 (see separate commits).I didn't remember what exactly this test was suppossed to test :-), so wasn't sure that
.to_sfix retains the test meaning, took me a couple days to understand & experiment...The
.to_sis fine, thelazy_finddone here is not the part tested.Turns out the test as I wrote it only checked the addition to
map_ids_to_inventory_objects. It still checks it — breaks without it.The addition to
update_or_destroy_records!isn't checked because test does just 1 refresh and all quota-related records are new.I can't simply wrap the test in
2.times do ... end, other parts break.=> I'll maybe try to change the test to pre-create one of quotas before refresh. But that would be a separate PR to master first.EDIT: however, this part is checked by manageiq-providers-kubernetes specs, so I think it's good enough as is.
https://bugzilla.redhat.com/show_bug.cgi?id=1559544 (gaprindashvili)