Skip to content
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

feat(tests): Added tests for the tfe block #96

Merged
merged 26 commits into from
Feb 6, 2024
Merged

Conversation

ghost
Copy link

@ghost ghost commented Nov 14, 2023

This PR resolves issue #90.

Whats has changed/added in this PR

  • Fixed a bug with the workspace_env_vars in the TFE block. When the module was trying to merge the variables for the TFC workspace, it was incorrectly attempting to merge content from an undefined variable var.tfe.env_vars.
  • Renamed all tests (in test folder) to start with test_
  • Created a test for the tfe block in the new offical terraform testing framework. docs. This is located inside the tests folder. See new readme on how to run the test
  • Added an output block in the tfe module to be able to test the creation of workspace variables
  • Changed the role_definitions module to allow not specifying scope. If no scope is provided it will use the current subscription as the default scope. This was done to prevent us for specifying a subscription ID in the tests.

@ghost ghost added bug Something isn't working enhancement New feature or request labels Nov 14, 2023
@ghost ghost marked this pull request as ready for review November 14, 2023 21:16
@ghost ghost temporarily deployed to E2E_testing November 14, 2023 21:29 — with GitHub Actions Inactive
@ghost ghost temporarily deployed to E2E_testing November 16, 2023 10:37 — with GitHub Actions Inactive
@ghost ghost temporarily deployed to E2E_testing November 16, 2023 12:26 — with GitHub Actions Inactive
@@ -3,7 +3,7 @@ resource "azurerm_role_definition" "user_created" {
role_definition_id = each.value.role_definition_id
name = each.value.name
description = each.value.description
scope = each.value.scope
scope = each.value.scope == null ? "/subscriptions/${data.azurerm_client_config.current.subscription_id}" : each.value.scope
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to discuss if we can set the default scope to the workload's resource group id. Are there any downsides to doing that @SanderBlom?

Copy link
Contributor

@SanderBlom SanderBlom Jan 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would probably be a better to reduce the scope to the workload resource group. I'll update the PR :)

@SanderBlom
Copy link
Contributor

I reached out to Hashicorp about this issue and this was the response I got back:


I completely understand your limitation, and we are working on enhancing how variables are done in the test framework. It might not happen asap, might be a few months, but we know this is a problem and intend on building the capability.

Please open a github issue and we will prioritize this. We intend on making many more changes to the test framework, so we will work on this for sure.

Omar Ismail
Senior Product Manager, Terraform Core
oismail@hashicorp.com

@SanderBlom
Copy link
Contributor

I have now opened an issue on this hashicorp/terraform#34534

@SanderBlom SanderBlom changed the title Added tests for the tfe block and fixed bug with workspace_env_vars Feat(Tests): Added tests for the tfe block and fixed bug with workspace_env_vars Jan 31, 2024
@kimfy kimfy changed the title Feat(Tests): Added tests for the tfe block and fixed bug with workspace_env_vars feat(Tests): Added tests for the tfe block and fixed bug with workspace_env_vars Feb 6, 2024
@kimfy kimfy changed the title feat(Tests): Added tests for the tfe block and fixed bug with workspace_env_vars feat(tests): Added tests for the tfe block and fixed bug with workspace_env_vars Feb 6, 2024
@SanderBlom SanderBlom changed the title feat(tests): Added tests for the tfe block and fixed bug with workspace_env_vars feat(tests): Added tests for the tfe block and fixed bug Feb 6, 2024
@SanderBlom SanderBlom changed the title feat(tests): Added tests for the tfe block and fixed bug feat(tests): Added tests for the tfe block Feb 6, 2024
… is automatically validated in terraform 1.6
@SanderBlom SanderBlom merged commit 0495772 into trunk Feb 6, 2024
6 checks passed
@SanderBlom SanderBlom deleted the 90-add-tests-tfe branch February 6, 2024 14:21
@SanderBlom SanderBlom mentioned this pull request Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants