Major test overhaul for runtimes and podman

.mockery.yaml

@@ -51,3 +51,12 @@ packages:
       dir: pkg/azure/mocks
     interfaces:
       Azure:
+  github.com/astronomer/astro-cli/airflow/runtimes:
+    config:
+      dir: airflow/runtimes/mocks
+      outpkg: mocks
+    interfaces:
+      OSChecker:
+      ContainerRuntime:
+      PodmanEngine:
+      DockerEngine:

airflow/runtimes/command_test.go

@@ -1,10 +1,21 @@
 package runtimes
 
 import (
+	"testing"
+
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/suite"
 )
 
-func (s *ContainerRuntimeSuite) TestCommandExecution() {
+type ContainerRuntimeCommandSuite struct {
+	suite.Suite
+}
+
+func TestContainerRuntimeCommand(t *testing.T) {
+	suite.Run(t, new(ContainerRuntimeCommandSuite))
+}
+
+func (s *ContainerRuntimeCommandSuite) TestCommandExecution() {
 	s.Run("Command executes successfully", func() {
 		cmd := &Command{
 			Command: "echo",

airflow/runtimes/container_runtime.go

@@ -10,7 +10,6 @@ import (
 	"github.com/briandowns/spinner"
 
 	"github.com/astronomer/astro-cli/config"
-	"github.com/astronomer/astro-cli/pkg/fileutil"
 	"github.com/astronomer/astro-cli/pkg/util"
 	"github.com/pkg/errors"
 )
@@ -49,44 +48,26 @@ func GetContainerRuntime() (ContainerRuntime, error) {
 	// Return the appropriate container runtime based on the binary discovered.
 	switch containerRuntime {
 	case docker:
-		return CreateDockerRuntime(new(DefaultDockerEngine)), nil
+		return CreateDockerRuntime(new(DefaultDockerEngine), new(DefaultOSChecker)), nil
 	case podman:
-		return CreatePodmanRuntime(new(DefaultPodmanEngine)), nil
+		return CreatePodmanRuntime(new(DefaultPodmanEngine), new(DefaultOSChecker)), nil
 	default:
 		return nil, errors.New(containerRuntimeNotFoundErrMsg)
 	}
 }
 
-// FileChecker interface defines a method to check if a file exists.
-// This is here mostly for testing purposes. This allows us to mock
-// around actually checking for binaries on a live system as that
-// would create inconsistencies across developer machines when
-// working with the unit tests.
-type FileChecker interface {
-	Exists(path string) bool
-}
-
-// OSFileChecker is a concrete implementation of FileChecker.
-type OSFileChecker struct{}
-
-// Exists checks if the file exists in the file system.
-func (f OSFileChecker) Exists(path string) bool {
-	exists, _ := fileutil.Exists(path, nil)
-	return exists
-}
-
 // FindBinary searches for the specified binary name in the provided $PATH directories,
 // using the provided FileChecker. It searches each specific path within the systems
 // $PATH environment variable for the binary concurrently and returns a boolean result
 // indicating if the binary was found or not.
-func FindBinary(pathEnv, binaryName string, checker FileChecker) bool {
+func FindBinary(pathEnv, binaryName string, checker FileChecker, osChecker OSChecker) bool {
 	// Split the $PATH variable into it's individual paths,
 	// using the OS specific path separator character.
 	paths := strings.Split(pathEnv, string(os.PathListSeparator))
 
 	// Although programs can be called without the .exe extension,
 	// we need to append it here when searching the file system.
-	if IsWindows() {
+	if osChecker.IsWindows() {
 		binaryName += ".exe"
 	}
 
@@ -150,7 +131,7 @@ var GetContainerRuntimeBinary = func() (string, error) {
 	// Get the $PATH environment variable.
 	pathEnv := os.Getenv("PATH")
 	for _, binary := range binaries {
-		if found := FindBinary(pathEnv, binary, OSFileChecker{}); found {
+		if found := FindBinary(pathEnv, binary, new(DefaultOSFileChecker), new(DefaultOSChecker)); found {
 			return binary, nil
 		}
 	}

airflow/runtimes/container_runtime_test.go

@@ -4,9 +4,9 @@ import (
 	"errors"
 	"testing"
 
-	"github.com/stretchr/testify/assert"
+	"github.com/astronomer/astro-cli/airflow/runtimes/mocks"
 
-	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/assert"
 
 	"github.com/stretchr/testify/suite"
 )
@@ -15,23 +15,13 @@ type ContainerRuntimeSuite struct {
 	suite.Suite
 }
 
-func TestConfig(t *testing.T) {
+func TestContainerRuntime(t *testing.T) {
 	suite.Run(t, new(ContainerRuntimeSuite))
 }
 
-// Mock for GetContainerRuntimeBinary
-type MockRuntimeChecker struct {
-	mock.Mock
-}
-
-func (m *MockRuntimeChecker) GetContainerRuntimeBinary() (string, error) {
-	args := m.Called()
-	return args.String(0), args.Error(1)
-}
-
 func (s *ContainerRuntimeSuite) TestGetContainerRuntime() {
 	s.Run("GetContainerRuntime_Docker", func() {
-		mockChecker := new(MockRuntimeChecker)
+		mockChecker := new(mocks.RuntimeChecker)
 		mockChecker.On("GetContainerRuntimeBinary").Return(docker, nil)
 
 		// Inject the mock and make sure we restore after the test.
@@ -47,7 +37,7 @@ func (s *ContainerRuntimeSuite) TestGetContainerRuntime() {
 	})
 
 	s.Run("GetContainerRuntime_Podman", func() {
-		mockChecker := new(MockRuntimeChecker)
+		mockChecker := new(mocks.RuntimeChecker)
 		mockChecker.On("GetContainerRuntimeBinary").Return(podman, nil)
 
 		// Inject the mock and make sure we restore after the test.
@@ -63,7 +53,7 @@ func (s *ContainerRuntimeSuite) TestGetContainerRuntime() {
 	})
 
 	s.Run("GetContainerRuntime_Error", func() {
-		mockChecker := new(MockRuntimeChecker)
+		mockChecker := new(mocks.RuntimeChecker)
 		mockChecker.On("GetContainerRuntimeBinary").Return("", errors.New(containerRuntimeNotFoundErrMsg))
 
 		// Inject the mock and make sure we restore after the test.
@@ -80,17 +70,6 @@ func (s *ContainerRuntimeSuite) TestGetContainerRuntime() {
 	})
 }
 
-// MockFileChecker is a mock implementation of FileChecker for tests.
-type MockFileChecker struct {
-	existingFiles map[string]bool
-}
-
-// Exists is just a mock for os.Stat(). In our test implementation, we just check
-// if the file exists in the list of mocked files for a given test.
-func (m MockFileChecker) Exists(path string) bool {
-	return m.existingFiles[path]
-}
-
 // TestGetContainerRuntimeBinary runs a suite of tests against GetContainerRuntimeBinary,
 // using the MockFileChecker defined above.
 func (s *ContainerRuntimeSuite) TestGetContainerRuntimeBinary() {
@@ -163,8 +142,8 @@ func (s *ContainerRuntimeSuite) TestGetContainerRuntimeBinary() {
 
 	for _, tt := range tests {
 		s.Run(tt.name, func() {
-			mockChecker := MockFileChecker{existingFiles: tt.mockFiles}
-			result := FindBinary(tt.pathEnv, tt.binary, mockChecker)
+			mockChecker := mocks.FileChecker{ExistingFiles: tt.mockFiles}
+			result := FindBinary(tt.pathEnv, tt.binary, mockChecker, new(DefaultOSChecker))
 			s.Equal(tt.expected, result)
 		})
 	}

airflow/runtimes/docker_engine.go

@@ -8,16 +8,8 @@ const (
 	dockerOpenNotice      = "We couldn't start the docker engine automatically. Please start it manually and try again."
 )
 
-// DockerEngine is a struct that contains the functions needed to initialize Docker.
-// The concrete implementation that we use is DefaultDockerEngine below.
-// When running the tests, we substitute the default implementation with a mock implementation.
-type DockerEngine interface {
-	IsRunning() (string, error)
-	Start() (string, error)
-}
-
 // DefaultDockerEngine is the default implementation of DockerEngine.
-// The concrete functions defined here are called from the InitializeDocker function below.
+// The concrete functions defined here are called from the initializeDocker function below.
 type DefaultDockerEngine struct{}
 
 func (d DefaultDockerEngine) IsRunning() (string, error) {

airflow/runtimes/docker_runtime.go

@@ -7,23 +7,32 @@ import (
 	"github.com/briandowns/spinner"
 )
 
+// DockerEngine is a struct that contains the functions needed to initialize Docker.
+// The concrete implementation that we use is DefaultDockerEngine below.
+// When running the tests, we substitute the default implementation with a mock implementation.
+type DockerEngine interface {
+	IsRunning() (string, error)
+	Start() (string, error)
+}
+
 // DockerRuntime is a concrete implementation of the ContainerRuntime interface.
 // When the docker binary is chosen, this implementation is used.
 type DockerRuntime struct {
-	Engine DockerEngine
+	Engine    DockerEngine
+	OSChecker OSChecker
 }
 
-func CreateDockerRuntime(engine DockerEngine) DockerRuntime {
-	return DockerRuntime{Engine: engine}
+func CreateDockerRuntime(engine DockerEngine, osChecker OSChecker) DockerRuntime {
+	return DockerRuntime{Engine: engine, OSChecker: osChecker}
 }
 
 // Initialize initializes the Docker runtime.
 // We only attempt to initialize Docker on Mac today.
 func (rt DockerRuntime) Initialize() error {
-	if !isMac() {
+	if !rt.OSChecker.IsMac() {
 		return nil
 	}
-	return rt.InitializeDocker(defaultTimeoutSeconds)
+	return rt.initializeDocker(defaultTimeoutSeconds)
 }
 
 func (rt DockerRuntime) Configure() error {
@@ -38,9 +47,9 @@ func (rt DockerRuntime) Kill() error {
 	return nil
 }
 
-// InitializeDocker initializes the Docker runtime.
+// initializeDocker initializes the Docker runtime.
 // It checks if Docker is running, and if it is not, it attempts to start it.
-func (rt DockerRuntime) InitializeDocker(timeoutSeconds int) error {
+func (rt DockerRuntime) initializeDocker(timeoutSeconds int) error {
 	// Initialize spinner.
 	timeout := time.After(time.Duration(timeoutSeconds) * time.Second)
 	ticker := time.NewTicker(time.Duration(tickNum) * time.Millisecond)

airflow/runtimes/docker_runtime_test.go

@@ -2,83 +2,87 @@ package runtimes
 
 import (
 	"fmt"
+	"testing"
+
+	"github.com/astronomer/astro-cli/airflow/runtimes/mocks"
+	"github.com/stretchr/testify/suite"
 
 	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/mock"
 )
 
-type MockDockerEngine struct {
-	mock.Mock
+var (
+	mockDockerEngine    *mocks.DockerEngine
+	mockDockerOSChecker *mocks.OSChecker
+)
+
+type DockerRuntimeSuite struct {
+	suite.Suite
 }
 
-func (d *MockDockerEngine) IsRunning() (string, error) {
-	args := d.Called()
-	return args.String(0), args.Error(1)
+func TestDockerRuntime(t *testing.T) {
+	suite.Run(t, new(DockerRuntimeSuite))
 }
 
-func (d *MockDockerEngine) Start() (string, error) {
-	args := d.Called()
-	return args.String(0), args.Error(1)
+func (s *DockerRuntimeSuite) SetupTest() {
+	// Reset some variables to defaults.
+	mockDockerEngine = new(mocks.DockerEngine)
+	mockDockerOSChecker = new(mocks.OSChecker)
 }
 
-func (s *ContainerRuntimeSuite) TestStartDocker() {
+func (s *DockerRuntimeSuite) TestStartDocker() {
 	s.Run("Docker is running, returns nil", func() {
-		// Create mock initializer.
-		mockDockerEngine := new(MockDockerEngine)
 		// Simulate that the initial `docker ps` succeeds and we exit early.
 		mockDockerEngine.On("IsRunning").Return("", nil).Once()
+		mockDockerOSChecker.On("IsMac").Return(true)
 		// Create the runtime with our mock engine.
-		rt := CreateDockerRuntime(mockDockerEngine)
+		rt := CreateDockerRuntime(mockDockerEngine, mockDockerOSChecker)
 		// Run our test and assert expectations.
-		err := rt.InitializeDocker(defaultTimeoutSeconds)
+		err := rt.Initialize()
 		assert.Nil(s.T(), err, "Expected no error when docker is running")
 		mockDockerEngine.AssertExpectations(s.T())
 	})
 
 	s.Run("Docker is not running, tries to start and waits", func() {
-		// Create mock initializer.
-		mockDockerEngine := new(MockDockerEngine)
 		// Simulate that the initial `docker ps` fails.
 		mockDockerEngine.On("IsRunning").Return("", fmt.Errorf("docker not running")).Once()
 		// Simulate that `open -a docker` succeeds.
 		mockDockerEngine.On("Start").Return("", nil).Once()
 		// Simulate that `docker ps` works after trying to open docker.
 		mockDockerEngine.On("IsRunning").Return("", nil).Once()
+		mockDockerOSChecker.On("IsMac").Return(true)
 		// Create the runtime with our mock engine.
-		rt := CreateDockerRuntime(mockDockerEngine)
+		rt := CreateDockerRuntime(mockDockerEngine, mockDockerOSChecker)
 		// Run our test and assert expectations.
-		err := rt.InitializeDocker(defaultTimeoutSeconds)
+		err := rt.Initialize()
 		assert.Nil(s.T(), err, "Expected no error when docker starts after retry")
 		mockDockerEngine.AssertExpectations(s.T())
 	})
 
 	s.Run("Docker fails to open", func() {
-		// Create mock initializer.
-		mockDockerEngine := new(MockDockerEngine)
 		// Simulate `docker ps` failing.
 		mockDockerEngine.On("IsRunning").Return("", fmt.Errorf("docker not running")).Once()
 		// Simulate `open -a docker` failing.
 		mockDockerEngine.On("Start").Return("", fmt.Errorf("failed to open docker")).Once()
+		mockDockerOSChecker.On("IsMac").Return(true)
 		// Create the runtime with our mock engine.
-		rt := CreateDockerRuntime(mockDockerEngine)
+		rt := CreateDockerRuntime(mockDockerEngine, mockDockerOSChecker)
 		// Run our test and assert expectations.
-		err := rt.InitializeDocker(defaultTimeoutSeconds)
+		err := rt.Initialize()
 		assert.Equal(s.T(), fmt.Errorf(dockerOpenNotice), err, "Expected timeout error")
 		mockDockerEngine.AssertExpectations(s.T())
 	})
 
 	s.Run("Docker open succeeds but check times out", func() {
-		// Create mock initializer.
-		mockDockerEngine := new(MockDockerEngine)
 		// Simulate `docker ps` failing continuously.
 		mockDockerEngine.On("IsRunning").Return("", fmt.Errorf("docker not running"))
 		// Simulate `open -a docker` failing.
 		mockDockerEngine.On("Start").Return("", nil).Once()
 		// Create the runtime with our mock engine.
-		rt := CreateDockerRuntime(mockDockerEngine)
+		rt := CreateDockerRuntime(mockDockerEngine, mockDockerOSChecker)
 		// Run our test and assert expectations.
+		// Call the helper method directly with custom timeout.
 		// Simulate the timeout after 1 second.
-		err := rt.InitializeDocker(1)
+		err := rt.initializeDocker(1)
 		assert.Equal(s.T(), fmt.Errorf(timeoutErrMsg), err, "Expected timeout error")
 		mockDockerEngine.AssertExpectations(s.T())
 	})

airflow/runtimes/file_checker.go

@@ -0,0 +1,21 @@
+package runtimes
+
+import "github.com/astronomer/astro-cli/pkg/fileutil"
+
+// FileChecker interface defines a method to check if a file exists.
+// This is here mostly for testing purposes. This allows us to mock
+// around actually checking for binaries on a live system as that
+// would create inconsistencies across developer machines when
+// working with the unit tests.
+type FileChecker interface {
+	Exists(path string) bool
+}
+
+// DefaultOSFileChecker is a concrete implementation of FileChecker.
+type DefaultOSFileChecker struct{}
+
+// Exists checks if the file exists in the file system.
+func (f DefaultOSFileChecker) Exists(path string) bool {
+	exists, _ := fileutil.Exists(path, nil)
+	return exists
+}