Fix test errors with :container_label_tag_mapping factory

[cben]

Alternative to ManageIQ/manageiq#16517

ManageIQ/manageiq#16501 added validation that ContainerLabelTagMapping has a Tag, appropriately named, and with a read_only Classification.

The UI has always created mappings satisfying all this (it's just now core started depending on these assumptions).

But this test didn't create Tag & Classification, only a ContainerLabelTagMapping, which no longer works:
https://travis-ci.org/ManageIQ/manageiq-ui-classic/jobs/305345808#L2351-L2381

=> Switch to recently added factory that ensures such Tag & Classification.

@miq-bot add-label test, gaprindashvili/yes
ManageIQ/manageiq#16501 is gaprindashvili/backported, and broke manageiq-ui-classic on gaprindashvili too.

@h-kataria @agrare @NickLaMuro what do you think?

[NickLaMuro]

I have no problem with this, but does this mean that the :container_label_tag_mapping Factory should just be deleted?

[h-kataria]

I like this one better, more simpler change.

[martinpovolny]

I have no problem with this, but does this mean that the :container_label_tag_mapping Factory should just be deleted?

I think that quite lately @skateman was adding a basic factory for every model.

[miq-bot]

Checked commit cben@0dcf2a1 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 🏆

[martinpovolny]

I also had to add:

devil - [~/Projects/manageiq-ui-classic] (master)$ git diff
diff --git a/spec/views/ops/_label_tag_mapping_form.html.haml_spec.rb b/spec/views/ops/_label_tag_mapping_form.html.haml_spec.rb
index 8382566..dee321e 100644
--- a/spec/views/ops/_label_tag_mapping_form.html.haml_spec.rb
+++ b/spec/views/ops/_label_tag_mapping_form.html.haml_spec.rb
@@ -1,4 +1,6 @@
 describe 'ops/_label_tag_mapping_form.html.haml' do
+  helper ApplicationHelper
+
   before do
     assign(:sb, :active_tab => 'settings_label_tag_mapping')
   end

To be able to run the individual spec.

[cben]

does this mean that the :container_label_tag_mapping Factory should just be deleted?

Excellent question. The problem is that the mapping model "supports" 2 use cases:

• any-value -> category mappings — this is the only kind UI ever allowed creating.
• specific-value -> specific tag mappings — this is dead code.

To test both kinds, the core container_label_tag_mapping_spec still uses the base :container_label_tag_mapping factory (all these uses specify a separately created :tag => ...).
Unfortunately most tests in container_label_tag_mapping_spec are written in terms of specific mappings, and many of them I can't just drop, need to rewrite in terms of any-value mappings.
I really really hope to kill specific mappings post-gaprindashvili.

Perhaps it's also possible to merge both into one factory, and specific-value tests will simply override the tag, I'll think about it.

[cben]

@martinpovolny should I include this ApplicationHelper line in this PR?

[martinpovolny]

@cben : we discussed shortly with @NickLaMuro on the gitter and the inclusion should be probably done for all the view specs possibly in spec/support/view_helper.rb or in spec/spec_helper.rb.

I think it can wait for another PR. We shoud get master back to green first.

[NickLaMuro]

Perhaps it's also possible to merge both into one factory, and specific-value tests will simply override the tag, I'll think about it.

tl;dr; we should merge this as is. Seems to do the trick.

My "can we delete it" comment was more something to ponder, not something that has to be done with this effort. I am cool with merging this as is, and we can address the Factory clean up at a later time. I just figured using it at all at this point would just not work now that the validation has been added, but I might be wrong (I admit to not fully groking the change that caused this bug in the first place).

[cben]

tl;dr of my above explanation: presently there are tests in core that successfully use the base factory, but they all specify a separately created :tag => ....

[NickLaMuro]

tl;dr of my above explanation: presently there are tests in core that successfully use the base factory, but they all specify a separately created :tag => ....

Ah! That was the missing piece for me. Fair enough, and further emphasizes that we will "fix this later".

[simaishi]

Gaprindashvili backport details:

$ git log -1
commit 8875886499ed342c78eecccbf79b83669c4f1e13
Author: Harpreet Kataria <[email protected]>
Date:   Tue Nov 21 16:16:27 2017 -0500

    Merge pull request #2788 from cben/tag-mapping-factory
    
    Fix test errors with :container_label_tag_mapping factory
    (cherry picked from commit 612e4ca75aadf37955fb1bec21c03f3dfab5ae7e)