feat(sfs): Improve acc tests with min/max config by Manuelvaas · Pull Request #1232 · stackitcloud/terraform-provider-stackit

Manuelvaas · 2026-02-17T11:36:19Z

Description

relates to STACKITTPR-441

Checklist

Issue was linked above
Code format was applied: make fmt
Examples were added / adjusted (see examples/ directory)
Docs are up-to-date: make generate-docs (will be checked by CI)
Unit tests got implemented or updated
Acceptance tests got implemented or updated (see e.g. here)
Unit tests are passing: make test (will be checked by CI)
No linter issues: make lint (will be checked by CI)

stackit/internal/services/sfs/sfs_acc_test.go

github-actions · 2026-02-25T03:54:40Z

This PR was marked as stale after 7 days of inactivity and will be closed after another 7 days of further inactivity. If this PR should be kept open, just add a comment, remove the stale label or push new commits to it.

github-actions · 2026-03-07T03:27:00Z

This PR was marked as stale after 7 days of inactivity and will be closed after another 7 days of further inactivity. If this PR should be kept open, just add a comment, remove the stale label or push new commits to it.

relates to STACKITTPR-441

github-actions · 2026-03-09T15:44:03Z

Merging this branch will increase overall coverage

Impacted Packages	Coverage Δ	🤖
github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/sfs	0.00% (ø)
github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/sfs/share	11.89% (+0.62%)	👍

Coverage by file

Changed files (no unit tests)

Changed File	Coverage Δ	Total	Covered	Missed	🤖
github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/sfs/share/datasource.go	22.39% (+1.18%)	67 (+1)	15 (+1)	52	👍
github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/sfs/share/resource.go	8.68% (+0.42%)	219 (+1)	19 (+1)	200	👍

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

Changed unit test files

github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/sfs/sfs_acc_test.go

marceljk · 2026-03-10T12:30:28Z

stackit/internal/services/sfs/testdata/share-max.tf

+  performance_class = "Standard"
+  size_gigabytes    = 512
+  ip_acl            = ["192.168.42.1/32"]
+  region            = "eu01"


Avoid hardcoded regions in the test configs, because this can potentially fail if we change something in the future. The default region is already configured in the SFSProviderConfig() and this should be enough. If someone would change there the region to e.g. "eu02", the test case would fail, because the resource pool would be still in eu02 and can't be used by the sfs_share resource.

marceljk · 2026-03-10T12:35:15Z

stackit/internal/services/sfs/sfs_acc_test.go

+		config.ObjectVariable(map[string]config.Variable{
+			"ip_acl": config.ListVariable(config.StringVariable(exportPolicyMaxIpAcl1), config.StringVariable(exportPolicyMaxIpAcl2)),
+			"order":  config.IntegerVariable(1),
+		}),


The rules have some more optional fields (description, read_only, set_uuid and super_user). They should be set in the max test as well.

You can also just add a second rule, which has these fields set and let the first rule like it is, in order to test if the default values are correctly set.

marceljk · 2026-03-10T13:06:35Z

stackit/internal/services/sfs/sfs_acc_test.go

+var testConfigShareVarsMin = config.Variables{
+	"project_id":                 config.StringVariable(testutil.ProjectId),
+	"name":                       config.StringVariable("tf-acc-" + acctest.RandStringFromCharSet(8, acctest.CharSetAlpha)),
+	"region":                     config.StringVariable("eu01"),


Looks like the region var isn't used in the config. For the min config I also wouldn't set it because it's optional and will fallback to the default value.
For the max tests this can be set with the value from testutil.Region. Otherwise the cleanup stuff will fail

marceljk · 2026-03-10T13:42:29Z

stackit/internal/services/sfs/sfs_acc_test.go

+					resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "project_id", testutil.ProjectId),
+					resource.TestCheckResourceAttrSet("stackit_sfs_export_policy.exportpolicy", "id"),
+					resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "name", testutil.ConvertConfigVariable(testConfigExportPolicyVarsMin["name"])),
+
+					resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "rules.#", "0"),


In the datasource step you doesn't need to check the resource because it was already done in the previous step. Instead you should check if all the datasource fields have the same value like it was in the resource defined.

The easiest way to do it is, to copy the checks from the create step and add the prefix "data." in all checks

marceljk · 2026-03-10T13:44:20Z

stackit/internal/services/sfs/sfs_acc_test.go

-		panic(fmt.Sprintf("cannot render template: %v", err))
-	}
-	return buffer.String()
+					resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "rules.#", "0"),


Add here a check if the policy_id and region is set

marceljk · 2026-03-10T13:49:10Z

stackit/internal/services/sfs/sfs_acc_test.go

+				Config:          fmt.Sprintf("%s\n%s", testutil.SFSProviderConfig(), resourceExportPolicyMinConfig),
+				Check: resource.ComposeAggregateTestCheckFunc(


It's possible to set which action we expect here, because so far we didn't checked during the update, if a replace was trigger or just an in-place update.
See example from iaas acc test:

terraform-provider-stackit/stackit/internal/services/iaas/iaas_acc_test.go

Lines 5040 to 5044 in 285c7e3

ConfigPlanChecks: resource.ConfigPlanChecks{

PreApply: []plancheck.PlanCheck{

plancheck.ExpectResourceAction("stackit_routing_table.routing_table", plancheck.ResourceActionUpdate),

},

},

To be honest, we haven't set this yet in many tests, but I think we should start doing it.

marceljk · 2026-03-10T13:52:12Z

stackit/internal/services/sfs/sfs_acc_test.go

+					resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "rules.0.ip_acl.0", exportPolicyMaxIpAcl1),
+					resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "rules.0.ip_acl.1", exportPolicyMaxIpAcl2),

 					resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "rules.0.read_only", "false"),


check for description field is missing

marceljk · 2026-03-10T13:52:49Z

stackit/internal/services/sfs/sfs_acc_test.go

 				Check: resource.ComposeAggregateTestCheckFunc(
-					resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "project_id", exportPolicyResource["project_id"]),
+					resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "project_id", testutil.ProjectId),
 					resource.TestCheckResourceAttrSet("stackit_sfs_export_policy.exportpolicy", "id"),


check for policy_id is missing

marceljk · 2026-03-10T13:53:56Z

stackit/internal/services/sfs/sfs_acc_test.go

 				Check: resource.ComposeAggregateTestCheckFunc(
-					resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "project_id", exportPolicyResource["project_id"]),
+					resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "project_id", testutil.ProjectId),
 					resource.TestCheckResourceAttrSet("stackit_sfs_export_policy.exportpolicy", "id"),
-					resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "name", exportPolicyResource["name"]),
-					// check rule
+					resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "name", testutil.ConvertConfigVariable(testConfigExportPolicyVarsMax["name"])),
+
 					resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "rules.#", "1"),
 					resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "rules.0.order", "1"),
 					resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "rules.0.ip_acl.#", "2"),
-					resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "rules.0.ip_acl.0", exportPolicyResource["ip_acl_1"]),
-					resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "rules.0.ip_acl.1", exportPolicyResource["ip_acl_2"]),
+					resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "rules.0.ip_acl.0", exportPolicyMaxIpAcl1),
+					resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "rules.0.ip_acl.1", exportPolicyMaxIpAcl2),

 					resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "rules.0.read_only", "false"),
 					resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "rules.0.set_uuid", "false"),
 					resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "rules.0.super_user", "true"),


most of the checks here are only checking the resource and not the actual datasource

marceljk · 2026-03-10T13:55:11Z

stackit/internal/services/sfs/sfs_acc_test.go

+				ConfigVariables: testConfigExportPolicyVarsMaxUpdated(),
+				Config:          fmt.Sprintf("%s\n%s", testutil.SFSProviderConfig(), resourceExportPolicyMaxConfig),
 				Check: resource.ComposeAggregateTestCheckFunc(
-					resource.TestCheckResourceAttr("stackit_sfs_export_policy.exportpolicy", "project_id", exportPolicyResource["project_id"]),


check for policy_id is missing and description in the rules

rubenhoenle reviewed Feb 17, 2026

View reviewed changes

stackit/internal/services/sfs/sfs_acc_test.go Outdated Show resolved Hide resolved

github-actions bot added Stale PR is marked as stale due to inactivity. and removed Stale PR is marked as stale due to inactivity. labels Feb 25, 2026

github-actions bot added the Stale PR is marked as stale due to inactivity. label Mar 7, 2026

Manuelvaas removed the Stale PR is marked as stale due to inactivity. label Mar 9, 2026

Manuelvaas marked this pull request as ready for review March 9, 2026 15:39

Manuelvaas requested a review from a team as a code owner March 9, 2026 15:39

Manuelvaas added 5 commits March 9, 2026 16:39

feat(sfs): Improve acc tests with min/max config

0bbffc7

relates to STACKITTPR-441

min/max testdata

05831fc

Added min/max tests

21594e2

fixed nil pointer and not-required arguments

72eb6a6

generated docs

92ea047

Manuelvaas force-pushed the feature/sfs-improve-acc-test branch from de6559e to 92ea047 Compare March 9, 2026 15:39

marceljk reviewed Mar 10, 2026

View reviewed changes

		Config: fmt.Sprintf("%s\n%s", testutil.SFSProviderConfig(), resourceExportPolicyMinConfig),
		Check: resource.ComposeAggregateTestCheckFunc(

	ConfigPlanChecks: resource.ConfigPlanChecks{
	PreApply: []plancheck.PlanCheck{
	plancheck.ExpectResourceAction("stackit_routing_table.routing_table", plancheck.ResourceActionUpdate),
	},
	},

Conversation

Manuelvaas commented Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

Uh oh!

github-actions bot commented Feb 25, 2026

Uh oh!

github-actions bot commented Mar 7, 2026

Uh oh!

github-actions bot commented Mar 9, 2026

Merging this branch will increase overall coverage

Changed files (no unit tests)

Changed unit test files

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Manuelvaas commented Feb 17, 2026 •

edited

Loading