Skip to content

Commit 630269f

Browse files
committed
Modify tests so that they pass on macOS, fix issue with leaking file
1 parent 36226a8 commit 630269f

File tree

3 files changed

+28
-35
lines changed

3 files changed

+28
-35
lines changed

cmd/ocm-backplane/console/console_test.go

+16-28
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package console
33
import (
44
"os"
55
"reflect"
6+
"runtime"
67

78
"github.com/golang/mock/gomock"
89
. "github.com/onsi/ginkgo"
@@ -326,26 +327,20 @@ var _ = Describe("console command", func() {
326327
})
327328

328329
Context("An container is created to run the console, prior to doing that we need to check if container distro is supported", func() {
329-
330-
ced := &dockerLinux{}
331-
cep := &podmanLinux{}
332-
333-
// Consider refactoring the following function as it isn't being used
334-
It("Check for the existance of a invalid container engine", func() {
335-
vld, err := validateContainerEngine("Foo Bar")
336-
Expect(vld).To(BeFalse())
337-
Expect(err).To(MatchError(ContainSubstring("container engine can only be one of")))
338-
})
339-
340-
// For some strange reason reflect of cei returns a pointer
341-
342330
It("In the case we explicitly specify Podman, the code should return support for Podman", func() {
343331
oldpath := createPathPodman()
344332
o := consoleOptions{}
345333
o.containerEngineFlag = PODMAN
346334
cei, err := o.getContainerEngineImpl()
347335
Expect(err).To(BeNil())
348-
Expect(reflect.TypeOf(cei) == reflect.TypeOf(cep)).To(BeTrue())
336+
337+
if runtime.GOOS == LINUX {
338+
Expect(reflect.TypeOf(cei) == reflect.TypeOf(&podmanLinux{})).To(BeTrue())
339+
}
340+
if runtime.GOOS == MACOS {
341+
Expect(reflect.TypeOf(cei) == reflect.TypeOf(&podmanMac{})).To(BeTrue())
342+
}
343+
349344
removePath(oldpath)
350345
})
351346

@@ -355,28 +350,21 @@ var _ = Describe("console command", func() {
355350
o.containerEngineFlag = DOCKER
356351
cei1, err1 := o.getContainerEngineImpl()
357352
Expect(err1).To(BeNil())
358-
Expect(reflect.TypeOf(cei1) == reflect.TypeOf(ced)).To(BeTrue())
359-
removePath(oldpath)
360-
})
361353

362-
It("Test if environment varible could be read by the code to identify what container engine to use", func() {
363-
oldpath := createPathPodman()
364-
o := consoleOptions{}
365-
os.Setenv("CONTAINER_ENGINE", PODMAN)
366-
o.containerEngineFlag = ""
367-
cei2, err2 := o.getContainerEngineImpl()
368-
Expect(err2).To(BeNil())
369-
Expect(reflect.TypeOf(cei2) == reflect.TypeOf(cep)).To(BeTrue())
354+
if runtime.GOOS == LINUX {
355+
Expect(reflect.TypeOf(cei1) == reflect.TypeOf(&dockerLinux{})).To(BeTrue())
356+
}
357+
if runtime.GOOS == MACOS {
358+
Expect(reflect.TypeOf(cei1) == reflect.TypeOf(&dockerMac{})).To(BeTrue())
359+
}
370360
removePath(oldpath)
371361
})
372362

373-
It("Test the situation where the environment varible is something else", func() {
363+
It("Test the situation where the environment variable is not a supported value", func() {
374364
o := consoleOptions{}
375365
o.containerEngineFlag = "FOO"
376-
os.Setenv("BACKPLANE_DEFAULT_OPEN_BROWSER", "FALSE")
377366
_, err4 := o.getContainerEngineImpl()
378367
Expect(err4).To(MatchError(ContainSubstring("container engine can only be one of podman|docker")))
379-
os.Setenv("BACKPLANE_DEFAULT_OPEN_BROWSER", "")
380368
})
381369
})
382370

pkg/elevate/elevate.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,8 @@ func RunElevate(argv []string) error {
6666
return err
6767
}
6868

69-
// As WriteKubeconfigToFile is also creating a tempory file reference by new KUBECONFIG variable setting,
70-
// we need to take care of its cleanup
69+
// As WriteKubeconfigToFile is also creating a temporary file referenced by new KUBECONFIG variable setting,
70+
// we need to take care of it's cleanup
7171
tempKubeconfigPath, _ := os.LookupEnv("KUBECONFIG")
7272
defer func() {
7373
logger.Debugln("Cleaning up temporary kubeconfig", tempKubeconfigPath)

pkg/elevate/elevate_test.go

+10-5
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,7 @@ var fakeAPIConfig = api.Config{
3939
},
4040
},
4141
AuthInfos: map[string]*api.AuthInfo{
42-
"anonymous": {
43-
LocationOfOrigin: "England",
44-
},
42+
"anonymous": {},
4543
},
4644
Contexts: map[string]*api.Context{
4745
"default/test123/anonymous": {
@@ -115,9 +113,16 @@ func TestAddElevationReasonToRawKubeconfig(t *testing.T) {
115113
}
116114

117115
func TestRunElevate(t *testing.T) {
118-
// We do ot want to realy override any config files or remove them
116+
// TODO: Utilize a fake filesystem for tests and stop relying on local system state
117+
tmpDir, err := os.MkdirTemp("", "backplane-cli-tests")
118+
if err != nil {
119+
t.Fatal(err)
120+
}
121+
defer os.RemoveAll(tmpDir)
119122

120-
OsRemove = func(name string) error { return nil }
123+
// We utilize the fact that elevate honors KUBECONFIG in order to not clobber the users real ~/.kube/config
124+
kubeconfigPath := tmpDir + "config"
125+
_ = os.Setenv("KUBECONFIG", kubeconfigPath)
121126

122127
t.Run("It returns an error if we cannot load the kubeconfig", func(t *testing.T) {
123128
ExecCmd = exec.Command

0 commit comments

Comments
 (0)