-
Notifications
You must be signed in to change notification settings - Fork 900
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
Improve tag mapping tests #16453
Improve tag mapping tests #16453
Conversation
There are tests in many places approximating this, not all correctly...
Use factory for more realistic setup. Add test for https://bugzilla.redhat.com/show_bug.cgi?id=1508437
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.
Looks good 👍
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.
❤️ this is awesome @cben
@cben are any of these dependent PRs going to be backported to gaprindashvili? |
Yes, all of them. I'll check the labels.
|
@miq-bot add-label fine/yes |
Improve tag mapping tests (cherry picked from commit 66a5465) https://bugzilla.redhat.com/show_bug.cgi?id=1510095
Gaprindashvili backport details:
|
In ManageIQ#16453, I changed this spec to "realistically" create a tag mapping + category + tag. But I made a typo/thinko, taking the parent (category) tag to be assigned to the vm. This caused false failure for ManageIQ#16449 and days of lost work debugging/rewriting it, eventually caused similar failure in the rewrite too :-( https://bugzilla.redhat.com/show_bug.cgi?id=1508437
@cben there is a conflict backporting to Fine branch in ++<<<<<<< HEAD
+ @tag1 = FactoryGirl.create(:tag, :name => '/managed/amazon:vm:owner')
+ @tag2 = FactoryGirl.create(:tag, :name => '/managed/kubernetes:container_node:stuff')
+ @tag3 = FactoryGirl.create(:tag, :name => '/managed/kubernetes:foo') # All
+
+ @cat1 = FactoryGirl.create(:category, :description => 'amazon_vm_owner', :tag => @tag1)
+ @cat2 = FactoryGirl.create(:category, :description => 'department', :tag => @tag2)
+ @cat3 = FactoryGirl.create(:category, :description => 'location', :tag => @tag3)
++=======
+ @tag1 = mapped_tag('amazon:vm:owner', 'alice')
+ @tag2 = mapped_tag('kubernetes:container_node:stuff', 'jabberwocky')
+ @tag3 = mapped_tag('kubernetes::foo', 'bar') # All entities
++>>>>>>> 66a5465... Merge pull request #16453 from cben/mapping_with_category_factory |
Backported to Fine via #16511 |
…tory Improve tag mapping tests
…tory Improve tag mapping tests
:tag_mapping_with_category
.There are tests in many places approximating this, not all correctly...
(tag mapping with graph refresh manageiq-providers-kubernetes#126)
https://bugzilla.redhat.com/show_bug.cgi?id=1508437 (master)
https://bugzilla.redhat.com/show_bug.cgi?id=1510095 (gaprindashvili)
https://bugzilla.redhat.com/show_bug.cgi?id=1510096 (fine)
@Ladas @agrare Please review this first, unblocks other PRs to not have circular deps...