From 609c5558899e36df8f0c9a980a290e0ee2d2de4b Mon Sep 17 00:00:00 2001
From: Ludovico Magnocavallo <ludomagno@google.com>
Date: Thu, 12 Sep 2019 08:13:14 +0200
Subject: [PATCH] refactor Python tests for style

---
 tests/foundations/business-units/test_folders.py  | 15 ++++++++++-----
 tests/foundations/business-units/test_outputs.py  | 14 +++++++-------
 tests/foundations/business-units/test_projects.py |  5 +++--
 3 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/tests/foundations/business-units/test_folders.py b/tests/foundations/business-units/test_folders.py
index 538b48427..22b05ab00 100644
--- a/tests/foundations/business-units/test_folders.py
+++ b/tests/foundations/business-units/test_folders.py
@@ -21,18 +21,23 @@ import pytest
 def test_shared_folder(plan):
   "Shared folder resource attributes must match variables."
   root_node = plan.variables['root_node']
-  resource = plan.modules['module.shared-folder']['module.shared-folder.google_folder.folders[0]']
+  resource = plan.modules['module.shared-folder'].get(
+      'module.shared-folder.google_folder.folders[0]')
   assert resource['values']['parent'] == root_node
   assert resource['values']['display_name'] == 'shared'
 
 
 def test_business_unit_folders(plan):
   "Business Unit folder resource attributes must match variables."
-  folder_resource_addresses = ['module.business-unit-%s-folders.module.business-unit-folder.google_folder.folders[0]' %
-          num for num in (1,2,3)]
+  address_tpl = (
+      'module.business-unit-%s-folders.module.business-unit-folder'
+      '.google_folder.folders[0]'
+  )
+  count = range(1, 4)
+  business_unit_names = [
+      plan.variables['business_unit_%s_name' % i] for i in count]
   root_node = plan.variables['root_node']
-  business_unit_names = [plan.variables[name] for name in ('business_unit_1_name', 'business_unit_2_name', 'business_unit_3_name')]
-  for address in folder_resource_addresses:
+  for address in [address_tpl % i for i in count]:
     resource = plan.resource_changes[address]
     assert resource['change']['after']['parent'] == root_node
     assert resource['change']['after']['display_name'] in business_unit_names
diff --git a/tests/foundations/business-units/test_outputs.py b/tests/foundations/business-units/test_outputs.py
index a24693bf1..c485b2e8c 100644
--- a/tests/foundations/business-units/test_outputs.py
+++ b/tests/foundations/business-units/test_outputs.py
@@ -18,9 +18,9 @@
 def test_project_ids(plan):
   "Project ids should use prefix and match expected values."
   prefix = plan.variables['prefix']
-  assert plan.outputs['audit_logs_project'] == prefix + '-audit'
-  assert plan.outputs['shared_resources_project'] == prefix + '-shared'
-  assert plan.outputs['terraform_project'] == prefix + '-terraform'
+  assert plan.outputs['audit_logs_project'] == '%s-audit' % prefix
+  assert plan.outputs['shared_resources_project'] == '%s-shared' % prefix
+  assert plan.outputs['terraform_project'] == '%s-terraform' % prefix
 
 
 def test_bucket_names(plan):
@@ -42,7 +42,7 @@ def test_environment_buckets(plan):
 
 
 def test_bq_dataset(plan):
-  "Bigquery dataset should be named in the following way 'logs_audit_<ROOT_NODE_TYPE>_<ROOT NODE NUMERIC ID>'"
-  root_node_type = plan.variables['root_node'].split("/")[0][:-1]
-  root_node_numeric_id = plan.variables['root_node'].split("/")[1]
-  assert plan.outputs['audit_logs_bq_dataset'] == 'logs_audit_' + root_node_type + '_' + root_node_numeric_id
+  "Bigquery dataset name should be based on root node type and id."
+  node_type, node_id = plan.variables['root_node'].split('/')
+  assert plan.outputs['audit_logs_bq_dataset'] == 'logs_audit_%s_%s' % (
+      node_type[:-1], node_id)
diff --git a/tests/foundations/business-units/test_projects.py b/tests/foundations/business-units/test_projects.py
index 0d2611cc8..87cb1a0c6 100644
--- a/tests/foundations/business-units/test_projects.py
+++ b/tests/foundations/business-units/test_projects.py
@@ -27,9 +27,10 @@ def project_modules(plan):
 
 def test_project_resource(plan, project_modules):
   "Project resource attributes must match variables."
+  names = ('shared', 'terraform', 'audit')
+  prefix = plan.variables['prefix']
   billing_account = plan.variables['billing_account_id']
-  project_names =  ['%s-%s' %
-           (plan.variables['prefix'], name) for name in ('shared', 'terraform', 'audit')] 
+  project_names = ['%s-%s' % (prefix, name) for name in names]
   for name, mod in project_modules.items():
     resource = mod['%s.google_project.project' % name]
     assert resource['values']['billing_account'] == billing_account