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

HCPIE-1244: Improve Identity-Specific Testing #810

Merged
merged 8 commits into from
Apr 16, 2024

Conversation

itsjaspermilan
Copy link
Contributor

@itsjaspermilan itsjaspermilan commented Apr 11, 2024

🛠️ Description

This PR aims to improve how identity-specific tests are run.

  • Ensure identity tests run within 10 minutes.
  • Make it easy to run identity tests locally. Added a section in the contributing.md docs that explains how to run a specific package.
  • Add a GitHub Action that only runs identity tests when identity-related files are changed.

I've added a comment line in one of the identity files to trigger the test workflow. This of course can't be merged to main so I've removed it now after confirming that the action did indeed run on a change. Reviewers can check the commit history to see the checks that passed when this comment was still present. Specifically this commit: https://github.com/hashicorp/terraform-provider-hcp/actions/runs/8649951887/job/23717371696?pr=810

Output from acceptance testing:

>make testacc TEST=./internal/provider/iam
==> Checking that code complies with gofmt requirements...
golangci-lint run --config ./golangci-config.yml 
TF_ACC=1 go test ./internal/provider/iam -v  -timeout 360m -parallel=10
=== RUN   TestAccGroupDataSource
--- PASS: TestAccGroupDataSource (8.69s)
=== RUN   TestAccServicePrincipalDataSource
--- PASS: TestAccServicePrincipalDataSource (5.76s)
=== RUN   TestAccUserPrincipalDataSource
--- PASS: TestAccUserPrincipalDataSource (8.79s)
=== RUN   TestAccGroupMembersResource
--- PASS: TestAccGroupMembersResource (24.92s)
=== RUN   TestAccGroupResource
--- PASS: TestAccGroupResource (15.57s)
=== RUN   TestAccServicePrincipalKeyResource
--- PASS: TestAccServicePrincipalKeyResource (24.06s)
=== RUN   TestAccServicePrincipalResource_Project
--- PASS: TestAccServicePrincipalResource_Project (11.57s)
=== RUN   TestAccServicePrincipalResource_ExplicitProject
--- PASS: TestAccServicePrincipalResource_ExplicitProject (6.01s)
=== RUN   TestAccServicePrincipalResource_Organization
--- PASS: TestAccServicePrincipalResource_Organization (7.68s)
=== RUN   TestAccWorkloadIdentityProviderResource
--- PASS: TestAccWorkloadIdentityProviderResource (11.67s)
PASS
ok      github.com/hashicorp/terraform-provider-hcp/internal/provider/iam       125.260s

@itsjaspermilan itsjaspermilan marked this pull request as ready for review April 11, 2024 16:25
@itsjaspermilan itsjaspermilan requested a review from a team as a code owner April 11, 2024 16:25
Copy link
Member

@squaresurf squaresurf left a comment

Choose a reason for hiding this comment

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

praise: great work on this! 🙌

@@ -0,0 +1,46 @@
name: Run Identity Tests on Identity Code Changes
Copy link
Contributor

Choose a reason for hiding this comment

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

This is awesome! Can this be extended to all the domains? As in run secrets test when those are changed? I had created ticket for this https://hashicorp.atlassian.net/browse/HCPF-1722

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@manish-hashicorp
Yes! It can be extended manually for sure if each team wants to add their own step to this workflow. If we wanted a more autonomous solution, we might have to spend a bit more time on it.

Roughly speaking, we could do the following:

  • Get a listing of all changed files in Git. Parse the list to only show the modified directories.
  • Get a recursive listing of all directories within ./internal
  • Loop through the lists and mark which directories to run tests for
  • Loop again and actually run the tests with the changed files using make testacc-ci TEST=DIRECTORY_NAME

This will be out of scope for this specific ticket since it will require some more testing, but with that being said, I'm happy to update the ticket you posted with my findings. That way, whoever picks it up will have a head start on how to complete the work.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks. Please do add information in that ticket.

@itsjaspermilan itsjaspermilan merged commit a6a5881 into main Apr 16, 2024
6 checks passed
@itsjaspermilan itsjaspermilan deleted the hcpie-1244-identity-tests-are-easier-to-run branch April 16, 2024 18:54
catsby added a commit that referenced this pull request Apr 17, 2024
* main:
  New Resource: Waypoint Add-on (#807)
  Gracefully handle rate limiting error on `hcp_vault_secrets_secret` resource. (#812)
  HCPIE-1244: Improve Identity-Specific Testing (#810)
  Fix code ownership for vault secrets files (#814)
  [COMPLIANCE] Add Copyright and License Headers (#799)
  update tests to use real tf no-code modules (#811)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants