fix: handled lint issue

[Hardikl]

No description provided.

[Copilot]

Pull request overview

This PR addresses a Go lint warning by replacing a non-formatting fmt.Errorf(...) with errors.New(...), and updates integration test expectations around iSCSI initiator removal.

Changes:

• Replace fmt.Errorf("...") (no formatting) with errors.New("...") in async job response handling.
• Modify TestIGroupLUNMap verification for “Remove initiator from igroup” steps to pass an empty expected initiator name.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
rest/client.go	Uses errors.New for a static error string to satisfy lint guidance.
integration/test/igroup_test.go	Adjusts initiator-removal verification parameters in integration tests.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

integration/test/igroup_test.go:88

• Passing an empty expected initiator name makes verifyInitiator(false, ...) a no-op: it will only fail if an initiator with an empty name exists, so the test can pass even if the IQN was not removed.

			verifyAPI:        ontapVerifier{api: "api/protocols/san/igroups?name=" + rn("igroupFin") + "&svm.name=" + rn("marketing") + "&fields=initiators", validationFunc: verifyInitiator(false, "")},

integration/test/igroup_test.go:100

• Same issue here: using an empty expected initiator name means the test does not actually assert that the specified IQN was removed from the igroup.

			verifyAPI:        ontapVerifier{api: "api/protocols/san/igroups?name=" + rn("igroupFin") + "&svm.name=" + rn("marketing") + "&fields=initiators", validationFunc: verifyInitiator(false, "")},

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (2)

integration/test/igroup_test.go:88

• verifyInitiator(false, "") makes the negative check ineffective: verifyInitiator only compares initiator.Name to expectedInitiatorName, so passing an empty string will pass even if the real initiator (iqn...) is still present.

			verifyAPI:        ontapVerifier{api: "api/protocols/san/igroups?name=" + rn("igroupFin") + "&svm.name=" + rn("marketing") + "&fields=initiators", validationFunc: verifyInitiator(false, "")},

integration/test/igroup_test.go:100

• verifyInitiator(false, "") makes the negative check ineffective: verifyInitiator only compares initiator.Name to expectedInitiatorName, so passing an empty string will pass even if the real initiator (iqn...) is still present.

			verifyAPI:        ontapVerifier{api: "api/protocols/san/igroups?name=" + rn("igroupFin") + "&svm.name=" + rn("marketing") + "&fields=initiators", validationFunc: verifyInitiator(false, "")},

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (2)

integration/test/igroup_test.go:88

• Passing an empty expectedInitiatorName means verifyInitiator(false, "") will not detect if the initiator is still present (it only checks for absence of the specific name). This weakens the test and can let regressions slip through.

			verifyAPI:        ontapVerifier{api: "api/protocols/san/igroups?name=" + rn("igroupFin") + "&svm.name=" + rn("marketing") + "&fields=initiators", validationFunc: verifyInitiator(false, "")},

integration/test/igroup_test.go:100

• Passing an empty expectedInitiatorName means verifyInitiator(false, "") will not detect if the initiator is still present (it only checks for absence of the specific name). This weakens the test and can let regressions slip through.

			verifyAPI:        ontapVerifier{api: "api/protocols/san/igroups?name=" + rn("igroupFin") + "&svm.name=" + rn("marketing") + "&fields=initiators", validationFunc: verifyInitiator(false, "")},

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

integration/test/igroup_test.go:89

• verifyInitiator(false, "") makes the check vacuous: verifyInitiator only looks for an exact name match, so passing an empty string will never detect that the initiator is still present. This subtest can pass even if removal failed.

			name:             "Remove initiator from igroup",
			input:            ClusterStr + "remove initiator iqn.2021-01.com.example:test from igroup " + rn("igroupFin") + " on the " + rn("marketing") + " svm",
			expectedOntapErr: "",
			verifyAPI:        ontapVerifier{api: "api/protocols/san/igroups?name=" + rn("igroupFin") + "&svm.name=" + rn("marketing") + "&fields=initiators", validationFunc: verifyInitiator(false, "")},
		},

integration/test/igroup_test.go:101

• Same issue as the earlier removal case: verifyInitiator(false, "") will not detect if the initiator is still present, so this test can pass incorrectly.

			name:             "Remove initiator from igroup",
			input:            ClusterStr + "remove initiator iqn.2021-01.com.example:test from igroup " + rn("igroupFin") + " on the " + rn("marketing") + " svm",
			expectedOntapErr: "",
			verifyAPI:        ontapVerifier{api: "api/protocols/san/igroups?name=" + rn("igroupFin") + "&svm.name=" + rn("marketing") + "&fields=initiators", validationFunc: verifyInitiator(false, "")},
		},

integration/test/qos_policy_test.go:84

• The //nolint:gocritic ... skipping these tests in CI comment is misleading: the subtests still execute ChatWithResponse for these cases; only the post-call API validation is skipped via verifyQoSAdaptiveFields(true, ...). Consider updating the comment to reflect what is actually being skipped.

		//nolint:gocritic These tests are available 9.10 onwards, current cluster is at 9.9, so skipping these tests in CI for now
		{
			name:             "Create adaptive QoS policy with allocation mode",
			input:            ClusterStr + "create an adaptive QoS policy named " + rn("alloc") + " on the marketing svm with expected iops 1000 peak iops 3000 absolute min iops 50 expected iops allocation allocated_space peak iops allocation used_space block size any",
			expectedOntapErr: "Unexpected argument",
			verifyAPI:        ontapVerifier{api: "api/storage/qos/policies?name=" + rn("alloc") + "&fields=adaptive", validationFunc: verifyQoSAdaptiveFields(true, "allocated_space", "used_space", "any")},
		},

integration/test/qos_policy_test.go:90

• verifyQoSAdaptiveFields is always invoked with skipTesting=true, so these subtests will never validate the adaptive fields even when running against an ONTAP 9.10+ cluster. If the intent is to skip only in CI/older clusters, this should be gated on cluster version (or CI env) instead of being hard-coded.

			name:             "Create adaptive QoS policy with allocation mode",
			input:            ClusterStr + "create an adaptive QoS policy named " + rn("alloc") + " on the marketing svm with expected iops 1000 peak iops 3000 absolute min iops 50 expected iops allocation allocated_space peak iops allocation used_space block size any",
			expectedOntapErr: "Unexpected argument",
			verifyAPI:        ontapVerifier{api: "api/storage/qos/policies?name=" + rn("alloc") + "&fields=adaptive", validationFunc: verifyQoSAdaptiveFields(true, "allocated_space", "used_space", "any")},
		},
		{
			name:             "Update adaptive QoS policy allocation mode only",
			input:            ClusterStr + "update the " + rn("alloc") + " QoS policy on the marketing svm to use allocated_space for both expected and peak IOPS allocation",
			expectedOntapErr: "because it does not exist",
			verifyAPI:        ontapVerifier{api: "api/storage/qos/policies?name=" + rn("alloc") + "&fields=adaptive", validationFunc: verifyQoSAdaptiveFields(true, "allocated_space", "allocated_space", "any")},
		},