Extract variable Tower data from tests #93

Glutexo · 2018-05-29T11:02:44Z

Some properties of the records expected in a Tower instance are not
entered directly by the user: ids, timestamps, external data… These
are subject to change with every population and cassette re-recording.

In such case manual update of expectations in specs was
required. Collection of this data to a single resource file allows to
change only this single file instead of hunting the right magic
literals in the specs examples.

Extracted from #79 for easier review. This part contains only changes to the specs. Goes along with #92.

@miq-bot add_reviewer @jameswnl

miq-bot · 2018-05-31T15:40:54Z

This pull request is not mergeable. Please rebase and repush.

Glutexo · 2018-06-05T11:26:19Z

#91 got merged. Rebased to resolve conflicts: Removed data for Tower v2 tests.

bronaghs · 2018-06-19T16:05:44Z

@Glutexo - Can you address the conflicts please so we can get this in.

Glutexo · 2018-06-20T12:43:11Z

Rebased, resolved. VCRs were re-recorded and because of that ids and counts in spec changed. If this gets merged, doing such and update would be easier for anybody. (@jameswnl in this case)

tumido · 2018-07-19T13:57:50Z

@jameswnl, can this one please get reviewed so we can slowly get to the point when Stepan's PRs are merged? 😉

jameswnl · 2018-07-20T20:28:21Z

spec/support/ansible_shared/automation_manager/event_catcher/stream.rb

@@ -11,6 +11,10 @@
                       :verify_ssl => false,).tap { |provider| provider.authentications << auth }
  end

+  let(:tower_data) { Spec::Support::TowerDataHelper.tower_data }


We should be able to do a YAML.load_file(TOWER_DATA_FILE) where the TOWER_DATA_FILE point to the yaml file.
This way we can eliminate that spec/support/tower_data_helper.rb file which will make @jrafanie 😄

Can either define TOWER_DATA_FILE in spec_helper.rb or even load it up there. But may be other spec experts can help here

jameswnl

Eliminate the need for spec/support/tower_data_helper.rb

jameswnl · 2018-07-20T20:29:43Z

This is great!

tumido

@Glutexo, I've placed some comments inline. I can't get proper data from the tower...

@jameswnl I've removed the helper class and replace it with a shared context fetching the same YAML file in the shared specs wherever it is required.

tumido · 2018-07-23T16:51:40Z

spec/support/ansible_shared/automation_manager/refresh_configuartion_script_source.rb

@@ -14,27 +14,38 @@
  end
  let(:manager_class) { manager_class }

+  let(:tower_data) { Spec::Support::TowerDataHelper.tower_data }
+
+  let(:targeted_refresh_id) { tower_data['items']['targeted_refresh']['id'] }


@Glutexo, where can I get the targeted_refresh project, please? I'm missing it in all of you PRs unfortunately. It seems this was never added to the rake job for Tower population. I was only able to find a removed mention in a spec in the original PR #79

This PR extracts the actual data used in the specs. I tried to map it as well as possible to the data created by the Rake task, but still they are more or less independent. If I recall correctly, the ids used here were different from the hello_repo and another_repo, so I created a separate entity for them in the YAML.

This can be resolved when merging with the other PR by using hello_repo instead and changing the data in this spec. This will however require re-recording of the VCRs, which is what I didn’t want to do here. It’s the inconsistency between specs I was talking about with you earlier. :(

tumido · 2018-07-23T16:55:04Z

spec/support/ansible_shared/automation_manager/refresh_configuartion_script_source.rb

+  let(:targeted_refresh_id) { tower_data['items']['targeted_refresh']['id'] }
+  let(:targeted_refresh_last_updated) { tower_data['items']['targeted_refresh']['last_updated'].utc }
+  let(:targeted_refresh_playbooks) { tower_data['items']['targeted_refresh']['playbooks'] }
+  let(:nonexistent_repo_id) { tower_data['items']['nonexistent_repo']['id'] }


The same case as the previous comment. This PR is the first one mentioning a nonexistent_repo. Is this the same as failed_repo?

As the name suggests, a nonexistent_repo is an invalid id. It was used to ensure that the refresher does not even try to load this repo when updating the targeted_refresh one. When merging with the Rake task changes, it would be necessary to create such id. That can be achieved by creating some repo and then deleting it, making sure the id is captured and it neither belongs to any existing project in Tower, nor going to be reused unexpectedly in the future.

miq-bot · 2018-07-27T10:38:50Z

This pull request is not mergeable. Please rebase and repush.

Glutexo

Hey, spotted some issues.

Glutexo · 2018-07-27T10:24:03Z

lib/tasks_private/spec_helper.rake

@@ -434,6 +434,20 @@ class PopulateTower
    last_update = wait_for_project_update(jobless_project)
    @conn.delete(last_update['url'])

+    # Create and remove project - record an collect an ID of missing entity
+    uri = '/api/v1/projects/'
+    uri = '/api/v1/projects/'


Dupy dupy dupy dup dup dup.

Glutexo · 2018-07-27T10:33:39Z

lib/tasks_private/spec_helper.rake

+    }
+    nonexistent_project = create_obj(uri, data)
+    @tower_data[:items][data[:name]] = { :id => nonexistent_project['id'] }
+    del_obj(uri, data[:name])


Just remember, that del_obj does some safety sleep. This might be necessary when recreating object that are already present in Tower. It is however useless here, when we just want to delete an object without any intention of creating its replacement.

7f4bdfe#diff-bb7272be93a294c53ae6433eb024074fR108

What's the issue with this? The rake job is run only in case of regenerating VCRs. It doesn't slow down the test run or anything...

You just wait a long time for nothing before going to the next step – populating item counts.

Glutexo · 2018-07-27T10:38:00Z

spec/support/ansible_shared/tower_data_helper.rb

@@ -0,0 +1,3 @@
+shared_context "uses tower_data.yml" do
+  let(:tower_data) { YAML.load_file(ManageIQ::Providers::AnsibleTower::Engine.root.join("spec/support/tower_data.yml")) }


I still don’t like this. Can’t we turn the path into a constant then use it when writing the file by the Rake task?

7f4bdfe#diff-bb7272be93a294c53ae6433eb024074fR11

Why would be a global constant better than a shared context? The context defines the test scope. Global constant would require the file to be loaded for any spec execution within this repo. A shared context allows lazy loading of the file - only if the spec requires it. In other words, this way it is not required to have the YAML present and loaded at all when you're running a spec which doesn't include the context.

There is nothing bad with the shared context. I’d just extract the file path to the global content. That is virtually no overhead (as compared to loading and parsing the whole file) for that and then in can be used even outside the specs for writing the file.

miq-bot · 2018-08-01T17:28:55Z

This pull request is not mergeable. Please rebase and repush.

Some properties of the records expected in a Tower instance are not entered directly by the user: ids, timestamps, external data… These are subject to change with every population and cassette re-recording. In such case manual update of expectations in specs was required. Collection of this data to a single resource file allows to change only this single file instead of hunting the right magic literals in the specs examples.

…spec

miq-bot · 2018-08-09T13:23:14Z

Some comments on commits Glutexo/manageiq-providers-ansible_tower@7043e91~...38f0f9a

lib/tasks_private/spec_helper.rake

⚠️ - 493 - Detected puts. Remove all debugging statements.
⚠️ - 498 - Detected puts. Remove all debugging statements.

miq-bot · 2018-08-09T13:23:20Z

Checked commits Glutexo/manageiq-providers-ansible_tower@7043e91~...38f0f9a with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
5 files checked, 0 offenses detected
Everything looks fine. 🍰

tumido · 2018-08-09T13:23:50Z

Should be mergeable again. @jameswnl can you please review and tell me your view on @Glutexo suggestions above? I'd say we pay too much attention to little tiny details, which are not relevant much.

tumido · 2018-08-09T14:09:53Z

@miq-bot remove_label unmergeable

tumido · 2018-08-10T13:36:57Z

@miq-bot add_label enhancement, test

Glutexo mentioned this pull request May 29, 2018

Collect variable Tower data upon population #92

Merged

miq-bot requested a review from jameswnl May 29, 2018 11:03

This was referenced May 29, 2018

[wip] Collect variable tower data #79

Closed

Capture object references when populating test data #78

Closed

miq-bot added the unmergeable label May 31, 2018

Glutexo force-pushed the extract_variable_tower_data branch from a18a58b to 86e4a8d Compare June 5, 2018 11:25

Glutexo force-pushed the extract_variable_tower_data branch from 86e4a8d to 0f817a0 Compare June 20, 2018 12:39

miq-bot removed the unmergeable label Jun 20, 2018

Glutexo mentioned this pull request Jun 21, 2018

Populate project without update job in Tower #82

Merged

1 task

bronaghs assigned jameswnl Jun 21, 2018

jameswnl reviewed Jul 20, 2018

View reviewed changes

jameswnl suggested changes Jul 20, 2018

View reviewed changes

tumido reviewed Jul 23, 2018

View reviewed changes

tumido force-pushed the extract_variable_tower_data branch from ab0f83f to d08a72c Compare July 23, 2018 17:22

jameswnl mentioned this pull request Jul 24, 2018

Add workflow in cassettes for refresh spec #111

Closed

tumido force-pushed the extract_variable_tower_data branch 2 times, most recently from 508c13a to 7f4bdfe Compare July 27, 2018 09:13

miq-bot added the unmergeable label Jul 27, 2018

Glutexo commented Jul 27, 2018

View reviewed changes

tumido force-pushed the extract_variable_tower_data branch 3 times, most recently from c2466bf to 02bff00 Compare July 27, 2018 11:31

miq-bot removed the unmergeable label Jul 27, 2018

miq-bot added the unmergeable label Aug 1, 2018

Glutexo and others added 4 commits August 9, 2018 14:14

Simplify tower_data.yml parser and update specs to reflect that

7823888

Update counters with {workflow_,}job_templates

6f93565

Update targeted_refresh VCR and spec to be consistant with refresher …

38f0f9a

…spec

tumido force-pushed the extract_variable_tower_data branch from 02bff00 to 38f0f9a Compare August 9, 2018 13:19

miq-bot removed the unmergeable label Aug 9, 2018

tumido mentioned this pull request Aug 9, 2018

Move ansible spec credentials to secret.yml #113

Closed

miq-bot added enhancement test labels Aug 10, 2018

tumido approved these changes Aug 10, 2018

View reviewed changes

jameswnl merged commit 7dfefd0 into ManageIQ:master Aug 10, 2018

Glutexo deleted the extract_variable_tower_data branch October 11, 2018 09:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extract variable Tower data from tests #93

Extract variable Tower data from tests #93

Glutexo commented May 29, 2018 •

edited

Loading

miq-bot commented May 31, 2018

Glutexo commented Jun 5, 2018

bronaghs commented Jun 19, 2018

Glutexo commented Jun 20, 2018

tumido commented Jul 19, 2018

jameswnl Jul 20, 2018

jameswnl left a comment

jameswnl commented Jul 20, 2018

tumido left a comment

tumido Jul 23, 2018

Glutexo Jul 24, 2018

tumido Jul 23, 2018

Glutexo Jul 24, 2018

miq-bot commented Jul 27, 2018

Glutexo left a comment

Glutexo Jul 27, 2018

Glutexo Jul 27, 2018

tumido Jul 27, 2018

Glutexo Jul 27, 2018 •

edited

Loading

Glutexo Jul 27, 2018

tumido Jul 27, 2018

Glutexo Jul 27, 2018

miq-bot commented Aug 1, 2018

miq-bot commented Aug 9, 2018

miq-bot commented Aug 9, 2018

tumido commented Aug 9, 2018

tumido commented Aug 9, 2018

tumido commented Aug 10, 2018

		@@ -0,0 +1,3 @@
		shared_context "uses tower_data.yml" do
		let(:tower_data) { YAML.load_file(ManageIQ::Providers::AnsibleTower::Engine.root.join("spec/support/tower_data.yml")) }

Extract variable Tower data from tests #93

Extract variable Tower data from tests #93

Conversation

Glutexo commented May 29, 2018 • edited Loading

miq-bot commented May 31, 2018

Glutexo commented Jun 5, 2018

bronaghs commented Jun 19, 2018

Glutexo commented Jun 20, 2018

tumido commented Jul 19, 2018

Choose a reason for hiding this comment

jameswnl left a comment

Choose a reason for hiding this comment

jameswnl commented Jul 20, 2018

tumido left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miq-bot commented Jul 27, 2018

Glutexo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Glutexo Jul 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miq-bot commented Aug 1, 2018

miq-bot commented Aug 9, 2018

miq-bot commented Aug 9, 2018

tumido commented Aug 9, 2018

tumido commented Aug 9, 2018

tumido commented Aug 10, 2018

Glutexo commented May 29, 2018 •

edited

Loading

Glutexo Jul 27, 2018 •

edited

Loading