address code review

Author: Tyler Slaton

Diff for: pkg/controller/operators/openshift/clusteroperator_controller_test.go

@@ -121,9 +121,7 @@ var _ = Describe("ClusterOperator controller", func() {
 		}))
 
 		By("setting upgradeable=false when there's an error determining compatibility")
-
-		By("changing the openshift environment variable to be invalid and restarting the ClusterOperator")
-		resetCurrentReleaseTo("")
+		Expect(resetCurrentReleaseTo("")).To(Succeed())
 		Eventually(func() error {
 			return k8sClient.Delete(ctx, co)
 		}).Should(Succeed())
@@ -139,8 +137,7 @@ var _ = Describe("ClusterOperator controller", func() {
 			LastTransitionTime: fixedNow(),
 		}))
 
-		By("setting an appropriate variable for the cluster version and restarting the ClusterOperator")
-		resetCurrentReleaseTo(clusterVersion)
+		Expect(resetCurrentReleaseTo(clusterVersion)).To(Succeed())
 		Eventually(func() error {
 			return k8sClient.Delete(ctx, co)
 		}).Should(Succeed())
@@ -306,10 +303,10 @@ var _ = Describe("ClusterOperator controller", func() {
 // resetCurrentRelease thread safely updates the currentRelease.version and then sets the openshift release
 // env var to the desired version. WARNING: This function should only be used for testing purposes as it
 // goes around the desired logic of only setting the version of the cluster for this operator once.
-func resetCurrentReleaseTo(version string) {
+func resetCurrentReleaseTo(version string) error {
 	currentRelease.mu.Lock()
 	defer currentRelease.mu.Unlock()
 
 	currentRelease.version = nil
-	os.Setenv(releaseEnvVar, version)
+	return os.Setenv(releaseEnvVar, version)
 }

Diff for: pkg/controller/operators/openshift/helpers_test.go

@@ -464,7 +464,7 @@ func TestIncompatibleOperators(t *testing.T) {
 			scheme := runtime.NewScheme()
 			require.NoError(t, AddToScheme(scheme))
 
-			fcli := fake.NewClientBuilder().WithObjects(objs...).WithScheme(scheme).Build()
+			fcli := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).Build()
 			incompatible, err := incompatibleOperators(context.Background(), fcli)
 			if tt.expect.err {
 				require.Error(t, err)