fix: update test #525

maheshwarishikha · 2023-09-05T06:11:41Z

Description

Updates test

Release required?

No release
Patch release (x.x.X)
Minor release (x.X.x)
Major release (X.x.x)

Release notes content

Run the pipeline

If the CI pipeline doesn't run when you create the PR, the PR requires a user with GitHub collaborators access to run the pipeline.

Run the CI pipeline when the PR is ready for review and you expect tests to pass. Add a comment to the PR with the following text:

/run pipeline

Checklist for reviewers

If relevant, a test for the change is included or updated with this PR.
If relevant, documentation for the change is included or updated with this PR.

Merge actions for mergers

When merging, use a relevant conventional commit message that is based on the PR contents and any release notes provided by the PR author. The commit message determines whether a new version of the module is needed, and if so, which semver increment to use (major, minor, or patch).
Merge by using "Squash and merge".

maheshwarishikha · 2023-09-05T08:10:09Z

/run pipeline

ocofaigh · 2023-09-05T08:42:59Z

@maheshwarishikha Did this test ever work? Wondering why all of a sudden we now need to add this import?

Also the test failed with this:
Error: [ERROR] Error creating authorization policy: The policy wasn't created because an access policy with identical attributes already exists. Please update the roles in the existing policy (30817f7e-a561-49ac-b01a-92f82fc4a45d), or update the one you're trying to assign to include a different attribute assignment.

This can happen due to the known limitation around scoping the auth policy. To get it to pass, you will have to do some cleanup on our account.

rajatagarwal-ibm · 2023-09-05T10:37:27Z

@maheshwarishikha we do not need this import as "testhelper" is already imported in the pr_test.go. Go lang shares imports at the same directory level.

maheshwarishikha · 2023-09-05T14:26:03Z

@maheshwarishikha we do not need this import as "testhelper" is already imported in the pr_test.go. Go lang shares imports at the same directory level.

When I run local as below, I hit the same error.

% make run-tests-local RUN=TestRunBasicExample      
cd tests && go test -run TestRunBasicExample -count=1 -v -timeout 300m
# github.com/terraform-ibm-modules/terraform-ibm-landing-zone-vsi [github.com/terraform-ibm-modules/terraform-ibm-landing-zone-vsi.test]
./other_test.go:12:13: undefined: testhelper
FAIL	github.com/terraform-ibm-modules/terraform-ibm-landing-zone-vsi [build failed]
make: *** [run-tests-local] Error 1

And after importing that library, error is resolved.

maheshwarishikha · 2023-09-05T14:38:48Z

/run pipeline

rajatagarwal-ibm · 2023-09-05T14:44:16Z

@maheshwarishikha I do not encounter that error.

ocofaigh · 2023-09-05T15:44:03Z

Maybe it is because you are explicitly running that test make run-tests-local RUN=TestRunBasicExample. Does it work if you just run make run-tests-local ?

rajatagarwal · 2023-09-05T15:59:50Z

@ocofaigh i ran exactly that same test

ocofaigh · 2023-09-05T16:19:02Z

@rajatagarwal I wonder would it work if you cleared your go cache and re-ran?

maheshwarishikha · 2023-09-05T16:39:04Z

Maybe it is because you are explicitly running that test make run-tests-local RUN=TestRunBasicExample. Does it work if you just run make run-tests-local ?

nopes...
it did not work for me either.

% make run-tests-local
cd tests && go test -run '' -count=1 -v -timeout 300m
# github.com/terraform-ibm-modules/terraform-ibm-landing-zone-vsi [github.com/terraform-ibm-modules/terraform-ibm-landing-zone-vsi.test]
./other_test.go:13:13: undefined: testhelper
FAIL	github.com/terraform-ibm-modules/terraform-ibm-landing-zone-vsi [build failed]
make: *** [run-tests-local] Error 1

Not sure, whats the difference between Rajat's and my m/c env.... 🤔

maheshwarishikha · 2023-09-05T16:52:13Z

/run pipeline

maheshwarishikha · 2023-09-06T06:02:40Z

/run pipeline

maheshwarishikha · 2023-09-06T06:17:43Z

@ocofaigh currently there is no existing policy in Dev account block storage -> HPCS but test is still failing with the error auth policy exists. Is it something related to upgrade test specifically? otherwise what else.... 🤔

rajatagarwal-ibm · 2023-09-06T08:50:08Z

@ocofaigh Regardless if we have that import in both files it would fail with an "unused import" error.

4197 ± make run-tests-local                                                                                                                          ✹
cd tests && go test -run '' -count=1 -v -timeout 300m
# github.com/terraform-ibm-modules/terraform-ibm-landing-zone-vsi [github.com/terraform-ibm-modules/terraform-ibm-landing-zone-vsi.test]
./other_test.go:7:2: "github.com/terraform-ibm-modules/ibmcloud-terratest-wrapper/testhelper" imported and not used
FAIL    github.com/terraform-ibm-modules/terraform-ibm-landing-zone-vsi [build failed]
make: *** [run-tests-local] Error 1

ocofaigh · 2023-09-06T09:04:33Z

@maheshwarishikha If the standard test and the upgrade test are trying to create the same auth policy, then yes we will face this issue. How about running one of the tests with key protect, and the other with hpcs so they don't clash?
We could update the example to handle both so we just need 1 example. but the tests trigger it differently

maheshwarishikha · 2023-09-06T10:43:35Z

@ocofaigh Regardless if we have that import in both files it would fail with an "unused import" error.

4197 ± make run-tests-local                                                                                                                          ✹
cd tests && go test -run '' -count=1 -v -timeout 300m
# github.com/terraform-ibm-modules/terraform-ibm-landing-zone-vsi [github.com/terraform-ibm-modules/terraform-ibm-landing-zone-vsi.test]
./other_test.go:7:2: "github.com/terraform-ibm-modules/ibmcloud-terratest-wrapper/testhelper" imported and not used
FAIL    github.com/terraform-ibm-modules/terraform-ibm-landing-zone-vsi [build failed]
make: *** [run-tests-local] Error 1

Are you trying out with the latest code? Because the new code added in other_test.go uses testhelper library.

ocofaigh · 2023-09-06T11:05:54Z

@maheshwarishikha I believe you are correct - I think this change is needed. Without it I get:

 % make run-tests-local RUN=TestRunBasicExample 
cd tests && go test -run TestRunBasicExample -count=1 -v -timeout 300m
# github.com/terraform-ibm-modules/terraform-ibm-landing-zone-vsi [github.com/terraform-ibm-modules/terraform-ibm-landing-zone-vsi.test]
./other_test.go:13:13: undefined: testhelper
FAIL	github.com/terraform-ibm-modules/terraform-ibm-landing-zone-vsi [build failed]
make: *** [run-tests-local] Error 1

maheshwarishikha · 2023-09-06T11:41:17Z

@maheshwarishikha If the standard test and the upgrade test are trying to create the same auth policy, then yes we will face this issue. How about running one of the tests with key protect, and the other with hpcs so they don't clash? We could update the example to handle both so we just need 1 example. but the tests trigger it differently

hmm..yes, that could be the reason. I'll check tests.

maheshwarishikha · 2023-09-11T07:15:35Z

@ocofaigh
I think example will handle any KMS. So as such no update required for example. One test can be configured with HPCS and another one with KMS. Will test it.

Got one question - Dont we use existing KP instance for any test? If yes, where are the details of KP instance stored? I do not find KP details at common-dev-assets/common-go-assets/common-permanent-resources.yaml

There is one instance geretain-keyprotect under geretain-vincent rg. Should we use that or create a new geretain instance?

…rraform-ibm-landing-zone-vsi into sm-5739

maheshwarishikha · 2023-09-11T12:00:43Z

/run pipeline

maheshwarishikha · 2023-09-11T12:24:17Z

@ocofaigh I think example will handle any KMS. So as such no update required for example. One test can be configured with HPCS and another one with KMS. Will test it.

Got one question - Dont we use existing KP instance for any test? If yes, where are the details of KP instance stored? I do not find KP details at common-dev-assets/common-go-assets/common-permanent-resources.yaml

There is one instance geretain-keyprotect under geretain-vincent rg. Should we use that or create a new geretain instance?

It is confirmed that PR tests were failing because of parallel execution. I just commented t.parallel to be sure for the problem, and the pipeline tests passed.
Regarding KP instance, confirmed with Daniel that we do not have any permanent KP instance to use and it is easy to create and use.
If we plan to run one test with HPCS and one with KP, then we need to modify example in such a way that

! kp_exists => create KP, rings get crn and run test

Alternate way is ...we can run tests sequentially instead of making example complex. This is what I tried and worked.

Let me know your thoughts on this. Thanks

ocofaigh · 2023-09-12T10:36:41Z

@maheshwarishikha We don't have a permanent Key Protect instance. We just create one in the examples. That way we always have a unique instance ID and full end to end examples. This also mans Key Protect auth policies won't clash since its a unique instance every time

ocofaigh · 2023-09-12T10:38:16Z

So can we have the standard test using HPCS and the upgrade test using Key Protect? That way there won't be any clashes

maheshwarishikha · 2023-09-12T14:34:23Z

So can we have the standard test using HPCS and the upgrade test using Key Protect? That way there won't be any clashes

It can be done. In this case, the example needs modification such that it will create KP instance, auth policy, keys etc if kms_does_not_exist already.

On a lighter note, how much it helps if we do this additional work in example just for PR test? why not to run test sequentially that also takes care of this clash policy issue?

ocofaigh · 2023-09-12T18:30:48Z

@maheshwarishikha I'm going to merge this to unblock the pipeline, however I still think we should re-work the example so it can be run in parallel. Otherwise it takes twice as long for pipeline to run tests

terraform-ibm-modules-ops · 2023-09-20T15:11:18Z

🎉 This PR is included in version 2.7.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

fix: update test

651a3ed

maheshwarishikha requested a review from rajatagarwal-ibm September 5, 2023 08:10

Merge branch 'main' into sm-5739

2f6328d

ShikhaMahe added 2 commits September 11, 2023 16:35

fix: update test

6590ffb

Merge branch 'sm-5739' of https://github.com/terraform-ibm-modules/te…

0de475f

…rraform-ibm-landing-zone-vsi into sm-5739

ocofaigh approved these changes Sep 12, 2023

View reviewed changes

ocofaigh merged commit 9ee8fe7 into main Sep 12, 2023

ocofaigh deleted the sm-5739 branch September 12, 2023 18:31

terraform-ibm-modules-ops added the released label Sep 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: update test #525

fix: update test #525

maheshwarishikha commented Sep 5, 2023 •

edited

Loading

maheshwarishikha commented Sep 5, 2023

ocofaigh commented Sep 5, 2023

rajatagarwal-ibm commented Sep 5, 2023 •

edited

Loading

maheshwarishikha commented Sep 5, 2023

maheshwarishikha commented Sep 5, 2023

rajatagarwal-ibm commented Sep 5, 2023

ocofaigh commented Sep 5, 2023

rajatagarwal commented Sep 5, 2023

ocofaigh commented Sep 5, 2023

maheshwarishikha commented Sep 5, 2023

maheshwarishikha commented Sep 5, 2023

maheshwarishikha commented Sep 6, 2023

maheshwarishikha commented Sep 6, 2023

rajatagarwal-ibm commented Sep 6, 2023

ocofaigh commented Sep 6, 2023 •

edited

Loading

maheshwarishikha commented Sep 6, 2023 •

edited

Loading

ocofaigh commented Sep 6, 2023

maheshwarishikha commented Sep 6, 2023

maheshwarishikha commented Sep 11, 2023 •

edited

Loading

maheshwarishikha commented Sep 11, 2023

maheshwarishikha commented Sep 11, 2023

ocofaigh commented Sep 12, 2023

ocofaigh commented Sep 12, 2023

maheshwarishikha commented Sep 12, 2023

ocofaigh commented Sep 12, 2023

terraform-ibm-modules-ops commented Sep 20, 2023

fix: update test #525

fix: update test #525

Conversation

maheshwarishikha commented Sep 5, 2023 • edited Loading

Description

Release required?

Release notes content

Run the pipeline

Checklist for reviewers

Merge actions for mergers

maheshwarishikha commented Sep 5, 2023

ocofaigh commented Sep 5, 2023

rajatagarwal-ibm commented Sep 5, 2023 • edited Loading

maheshwarishikha commented Sep 5, 2023

maheshwarishikha commented Sep 5, 2023

rajatagarwal-ibm commented Sep 5, 2023

ocofaigh commented Sep 5, 2023

rajatagarwal commented Sep 5, 2023

ocofaigh commented Sep 5, 2023

maheshwarishikha commented Sep 5, 2023

maheshwarishikha commented Sep 5, 2023

maheshwarishikha commented Sep 6, 2023

maheshwarishikha commented Sep 6, 2023

rajatagarwal-ibm commented Sep 6, 2023

ocofaigh commented Sep 6, 2023 • edited Loading

maheshwarishikha commented Sep 6, 2023 • edited Loading

ocofaigh commented Sep 6, 2023

maheshwarishikha commented Sep 6, 2023

maheshwarishikha commented Sep 11, 2023 • edited Loading

maheshwarishikha commented Sep 11, 2023

maheshwarishikha commented Sep 11, 2023

ocofaigh commented Sep 12, 2023

ocofaigh commented Sep 12, 2023

maheshwarishikha commented Sep 12, 2023

ocofaigh commented Sep 12, 2023

terraform-ibm-modules-ops commented Sep 20, 2023

maheshwarishikha commented Sep 5, 2023 •

edited

Loading

rajatagarwal-ibm commented Sep 5, 2023 •

edited

Loading

ocofaigh commented Sep 6, 2023 •

edited

Loading

maheshwarishikha commented Sep 6, 2023 •

edited

Loading

maheshwarishikha commented Sep 11, 2023 •

edited

Loading