magic-modules icon indicating copy to clipboard operation
magic-modules copied to clipboard

fix TestAccAccessApprovalSettings

Open shuyama1 opened this issue 1 year ago • 2 comments

Release Note Template for Downstream PRs (will be copied)


shuyama1 avatar Oct 16 '24 23:10 shuyama1

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 3 files changed, 27 insertions(+)) google-beta provider: Diff ( 3 files changed, 27 insertions(+))

modular-magician avatar Oct 16 '24 23:10 modular-magician

Tests analytics

Total tests: 4 Passed tests: 4 Skipped tests: 0 Affected tests: 0

Click here to see the affected service packages
  • accessapproval

🟢 All tests passed!

View the build log

modular-magician avatar Oct 16 '24 23:10 modular-magician

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 3 files changed, 27 insertions(+)) google-beta provider: Diff ( 3 files changed, 27 insertions(+))

modular-magician avatar Oct 21 '24 23:10 modular-magician

Tests analytics

Total tests: 4 Passed tests: 4 Skipped tests: 0 Affected tests: 0

Click here to see the affected service packages
  • accessapproval

🟢 All tests passed!

View the build log

modular-magician avatar Oct 22 '24 00:10 modular-magician

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 3 files changed, 27 insertions(+)) google-beta provider: Diff ( 3 files changed, 27 insertions(+))

modular-magician avatar Oct 22 '24 20:10 modular-magician

Tests analytics

Total tests: 4 Passed tests: 4 Skipped tests: 0 Affected tests: 0

Click here to see the affected service packages
  • accessapproval

🟢 All tests passed!

View the build log

modular-magician avatar Oct 22 '24 20:10 modular-magician

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 3 files changed, 27 insertions(+)) google-beta provider: Diff ( 3 files changed, 27 insertions(+))

modular-magician avatar Oct 22 '24 21:10 modular-magician

Tests analytics

Total tests: 4 Passed tests: 4 Skipped tests: 0 Affected tests: 0

Click here to see the affected service packages
  • accessapproval

🟢 All tests passed!

View the build log

modular-magician avatar Oct 22 '24 21:10 modular-magician

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 3 files changed, 27 insertions(+)) google-beta provider: Diff ( 3 files changed, 27 insertions(+))

modular-magician avatar Oct 22 '24 21:10 modular-magician

Tests analytics

Total tests: 4 Passed tests: 4 Skipped tests: 0 Affected tests: 0

Click here to see the affected service packages
  • accessapproval

🟢 All tests passed!

View the build log

modular-magician avatar Oct 22 '24 22:10 modular-magician

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 3 files changed, 27 insertions(+)) google-beta provider: Diff ( 3 files changed, 27 insertions(+))

modular-magician avatar Oct 22 '24 22:10 modular-magician

Tests analytics

Total tests: 4 Passed tests: 4 Skipped tests: 0 Affected tests: 0

Click here to see the affected service packages
  • accessapproval

🟢 All tests passed!

View the build log

modular-magician avatar Oct 22 '24 22:10 modular-magician

I'd be inclined to say let's go right to 2min if the expectation is ~90secs

melinath avatar Dec 16 '24 16:12 melinath

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 3 files changed, 27 insertions(+)) google-beta provider: Diff ( 3 files changed, 27 insertions(+))

modular-magician avatar Dec 16 '24 19:12 modular-magician

Tests analytics

Total tests: 4 Passed tests: 4 Skipped tests: 0 Affected tests: 0

Click here to see the affected service packages
  • accessapproval

🟢 All tests passed!

View the build log

modular-magician avatar Dec 16 '24 19:12 modular-magician

Run tests on TC and it failed with the following error:

------- Stdout: -------
=== RUN   TestAccAccessApprovalSettings
=== RUN   TestAccAccessApprovalSettings/folder
=== RUN   TestAccAccessApprovalSettings/project
=== RUN   TestAccAccessApprovalSettings/organization
    resource_access_approval_organization_settings_test.go:46: Step 5/6 error: Error running apply: exit status 1
        Error: Provider produced inconsistent result after apply
        When applying changes to
        google_organization_access_approval_settings.organization_access_approval,
        provider "provider[\"registry.terraform.io/hashicorp/google\"]" produced an
        unexpected new value: Root object was present, but now absent.
        This is a bug in the provider, which should be reported in the provider's own
        issue tracker.
--- FAIL: TestAccAccessApprovalSettings (1335.90s)
    --- PASS: TestAccAccessApprovalSettings/folder (543.34s)
    --- PASS: TestAccAccessApprovalSettings/project (399.34s)
    --- FAIL: TestAccAccessApprovalSettings/organization (393.21s)
FAIL

Looks like 2 minutes might not always be sufficient. In earlier attempts, I tested several wait times, and the tests only passed with a 5-minute wait. But adding such a long wait to the resource itself doesn’t feel right since it would unnecessarily slow down every creation and update for users. I’m thinking of switching approaches by adding a sleep delay in the test itself instead of the resource.

shuyama1 avatar Dec 16 '24 20:12 shuyama1

I don't think we should add the sleep delay to the test itself - this is a real issue that could impact users, since we read immediately after apply. If 5min worked we should just do that. It looks like the test is able to get a passing result, then still fail later, so we can't rely on a successful GET request to determine whether enough time has passed.

melinath avatar Dec 16 '24 22:12 melinath

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 3 files changed, 27 insertions(+)) google-beta provider: Diff ( 3 files changed, 27 insertions(+))

modular-magician avatar Dec 17 '24 00:12 modular-magician

Tests analytics

Total tests: 4 Passed tests: 4 Skipped tests: 0 Affected tests: 0

Click here to see the affected service packages
  • accessapproval

🟢 All tests passed!

View the build log

modular-magician avatar Dec 17 '24 00:12 modular-magician

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 3 files changed, 27 insertions(+)) google-beta provider: Diff ( 3 files changed, 27 insertions(+))

modular-magician avatar Dec 17 '24 04:12 modular-magician

Tests analytics

Total tests: 4 Passed tests: 4 Skipped tests: 0 Affected tests: 0

Click here to see the affected service packages
  • accessapproval

🟢 All tests passed!

View the build log

modular-magician avatar Dec 17 '24 05:12 modular-magician

/gcbrun

shuyama1 avatar Dec 18 '24 18:12 shuyama1

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 3 files changed, 27 insertions(+)) google-beta provider: Diff ( 3 files changed, 27 insertions(+))

modular-magician avatar Dec 18 '24 18:12 modular-magician

Tests analytics

Total tests: 4 Passed tests: 4 Skipped tests: 0 Affected tests: 0

Click here to see the affected service packages
  • accessapproval

🟢 All tests passed!

View the build log

modular-magician avatar Dec 18 '24 18:12 modular-magician

Sorry, forgot to update the test status here: Run test on TC and it passed only with a 5-min wait time. So let’s set it to 5-min for now, and we can reduce it in the future if the backend update time decreases.

shuyama1 avatar Dec 19 '24 19:12 shuyama1