Correct reading of state during update #125

[Noel-Jones]

Community Note

• Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
• Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Relates #125

[hashicorp-cla]

All committers have signed the CLA.

[SBGoods]

Thank you for the contribution, @Noel-Jones!
This is looking good code-wise, but could you also ensure there is a covering acceptance test? This helps us prevent potential regressions in the future. An example test might apply a configuration with one rotation value as a first step, then update the rotation values as a second step. Something like this in internal/provider/resource_time_rotating_test.go might get you close:

func TestAccTimeRotating_RotationDays_ToRotationMonths(t *testing.T) {
	resourceName := "time_rotating.test"
	timestamp := time.Now().UTC()

	resource.UnitTest(t, resource.TestCase{
		ProtoV5ProviderFactories: protoV5ProviderFactories(),
		CheckDestroy:             nil,
		Steps: []resource.TestStep{
			{
				Config: testAccConfigTimeRotatingRotationDays(timestamp.Format(time.RFC3339), 7),
				Check: resource.ComposeTestCheckFunc(
					resource.TestCheckResourceAttr(resourceName, "rotation_days", "7"),
					resource.TestCheckResourceAttr(resourceName, "rotation_rfc3339", timestamp.AddDate(0, 0, 7).Format(time.RFC3339)),
					resource.TestCheckNoResourceAttr(resourceName, "rotation_years"),
					resource.TestCheckNoResourceAttr(resourceName, "rotation_months"),
					resource.TestCheckNoResourceAttr(resourceName, "rotation_hours"),
					resource.TestCheckNoResourceAttr(resourceName, "rotation_minutes"),
					resource.TestCheckResourceAttrSet(resourceName, "rfc3339"),
				),
			},
			{
				Config: testAccConfigTimeRotatingRotationMonths(timestamp.Format(time.RFC3339), 3),
				Check: resource.ComposeTestCheckFunc(
					resource.TestCheckResourceAttr(resourceName, "rotation_months", "3"),
					resource.TestCheckResourceAttr(resourceName, "rotation_rfc3339", timestamp.AddDate(0, 3, 0).Format(time.RFC3339)),
					resource.TestCheckNoResourceAttr(resourceName, "rotation_years"),
					resource.TestCheckNoResourceAttr(resourceName, "rotation_days"),
					resource.TestCheckNoResourceAttr(resourceName, "rotation_hours"),
					resource.TestCheckNoResourceAttr(resourceName, "rotation_minutes"),
					resource.TestCheckResourceAttrSet(resourceName, "rfc3339"),
				),
			},
		},
	})
}

The testing can than be run with make testacc or TF_ACC=1 go test -count=1 -run='' -v ./internal/provider. It should hopefully not pass without the update and pass with the update.
Please reach out if you are not able to add this or have any questions. Thanks again!

[Noel-Jones]

Thanks Selena. I'd wanted to add a test but still feeling my way around the way it all works. I appreciate the full detail and partuculary the (now obvious) way to run them! Ran the test before and after. Before results were a fail:

=== RUN   TestAccTimeRotating_RotationDays_ToRotationMonths
    resource_time_rotating_test.go:396: Step 2/2 error: Error running apply: exit status 1

        Error: Provider produced inconsistent result after apply

        When applying changes to time_rotating.test, provider
        "provider[\"registry.terraform.io/hashicorp/time\"]" produced an unexpected
        new value: .rotation_days: was null, but now cty.NumberIntVal(7).

        This is a bug in the provider, which should be reported in the provider's own
        issue tracker.

        Error: Provider produced inconsistent result after apply

        When applying changes to time_rotating.test, provider
        "provider[\"registry.terraform.io/hashicorp/time\"]" produced an unexpected
        new value: .rotation_months: was cty.NumberIntVal(3), but now null.

        This is a bug in the provider, which should be reported in the provider's own
        issue tracker.

        Error: Provider produced inconsistent result after apply

        When applying changes to time_rotating.test, provider
        "provider[\"registry.terraform.io/hashicorp/time\"]" produced an unexpected
        new value: .day: was cty.NumberIntVal(21), but now cty.NumberIntVal(28).

        This is a bug in the provider, which should be reported in the provider's own
        issue tracker.

        Error: Provider produced inconsistent result after apply

        When applying changes to time_rotating.test, provider
        "provider[\"registry.terraform.io/hashicorp/time\"]" produced an unexpected
        new value: .month: was cty.NumberIntVal(1), but now cty.NumberIntVal(10).

        This is a bug in the provider, which should be reported in the provider's own
        issue tracker.

        Error: Provider produced inconsistent result after apply

        When applying changes to time_rotating.test, provider
        "provider[\"registry.terraform.io/hashicorp/time\"]" produced an unexpected
        new value: .unix: was cty.NumberIntVal(1.674287666e+09), but now
        cty.NumberIntVal(1.666943666e+09).

        This is a bug in the provider, which should be reported in the provider's own
        issue tracker.

        Error: Provider produced inconsistent result after apply

        When applying changes to time_rotating.test, provider
        "provider[\"registry.terraform.io/hashicorp/time\"]" produced an unexpected
        new value: .rotation_rfc3339: was cty.StringVal("2023-01-21T07:54:26Z"), but
        now cty.StringVal("2022-10-28T07:54:26Z").

        This is a bug in the provider, which should be reported in the provider's own
        issue tracker.

        Error: Provider produced inconsistent result after apply

        When applying changes to time_rotating.test, provider
        "provider[\"registry.terraform.io/hashicorp/time\"]" produced an unexpected
        new value: .year: was cty.NumberIntVal(2023), but now cty.NumberIntVal(2022).

        This is a bug in the provider, which should be reported in the provider's own
        issue tracker.
--- FAIL: TestAccTimeRotating_RotationDays_ToRotationMonths (1.96s)

[kevinbasoftcat]

Please can we get an update on when this will be released into v0.9.0?

[bookshelfdave]

@kevinbasoftcat we're planning on releasing early next week

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.