Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

test: mocks LimaUser to fix race condition in support bundle unit tests #450

Merged
merged 1 commit into from
Jun 13, 2023
Merged

test: mocks LimaUser to fix race condition in support bundle unit tests #450

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions cmd/finch/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ import (
"io"
"os"

"github.com/spf13/afero"
"github.com/spf13/cobra"

"github.com/runfinch/finch/pkg/command"
"github.com/runfinch/finch/pkg/config"
"github.com/runfinch/finch/pkg/dependency"
Expand All @@ -17,13 +20,11 @@ import (
"github.com/runfinch/finch/pkg/flog"
"github.com/runfinch/finch/pkg/fmemory"
"github.com/runfinch/finch/pkg/fssh"
"github.com/runfinch/finch/pkg/lima/wrapper"
"github.com/runfinch/finch/pkg/path"
"github.com/runfinch/finch/pkg/support"
"github.com/runfinch/finch/pkg/system"
"github.com/runfinch/finch/pkg/version"

"github.com/spf13/afero"
"github.com/spf13/cobra"
)

const finchRootCmd = "finch"
Expand Down Expand Up @@ -94,6 +95,7 @@ var newApp = func(logger flog.Logger, fp path.Finch, fs afero.Fs, fc *config.Fin
support.NewBundleConfig(fp, system.NewStdLib().Env("HOME")),
fp,
ecc,
wrapper.NewLimaWrapper(),
)

// append nerdctl commands
Expand Down
30 changes: 30 additions & 0 deletions pkg/lima/wrapper/lima_wrapper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

// Package wrapper provides an interface to wrap Lima utils
package wrapper

import (
"os/user"

"github.com/lima-vm/lima/pkg/osutil"
)

// LimaWrapper provides Lima utils in an interface to facilitate mocking
//
//go:generate mockgen --destination=../../mocks/lima_wrapper.go -package=mocks github.com/runfinch/finch/pkg/lima/wrapper LimaWrapper
type LimaWrapper interface {
LimaUser(warn bool) (*user.User, error)
}

type limaWrapper struct{}

// NewLimaWrapper returns a new LimaWrapper.
func NewLimaWrapper() LimaWrapper {
return &limaWrapper{}
}

// LimaUser returns the user that will be used inside the Lima VM.
func (*limaWrapper) LimaUser(warn bool) (*user.User, error) {
return osutil.LimaUser(warn)
}
50 changes: 50 additions & 0 deletions pkg/mocks/lima_wrapper.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 5 additions & 2 deletions pkg/support/support.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@ import (
"strings"
"time"

"github.com/lima-vm/lima/pkg/osutil"
"github.com/spf13/afero"
"gopkg.in/yaml.v3"

"github.com/runfinch/finch/pkg/command"
"github.com/runfinch/finch/pkg/flog"
"github.com/runfinch/finch/pkg/lima/wrapper"
fpath "github.com/runfinch/finch/pkg/path"
"github.com/runfinch/finch/pkg/version"
)
Expand Down Expand Up @@ -51,6 +51,7 @@ type bundleBuilder struct {
config BundleConfig
finch fpath.Finch
ecc command.Creator
lima wrapper.LimaWrapper
}

// NewBundleBuilder produces a new BundleBuilder.
Expand All @@ -60,13 +61,15 @@ func NewBundleBuilder(
config BundleConfig,
finch fpath.Finch,
ecc command.Creator,
lima wrapper.LimaWrapper,
) BundleBuilder {
return &bundleBuilder{
logger: logger,
fs: fs,
config: config,
finch: finch,
ecc: ecc,
lima: lima,
}
}

Expand Down Expand Up @@ -171,7 +174,7 @@ func (bb *bundleBuilder) copyInFile(writer *zip.Writer, fileName string, prefix
return err
}

user, err := osutil.LimaUser(false)
user, err := bb.lima.LimaUser(false)
if err != nil {
return err
}
Expand Down
64 changes: 56 additions & 8 deletions pkg/support/support_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package support

import (
"archive/zip"
"os/user"
"testing"
"time"

Expand All @@ -25,23 +26,34 @@ func TestSupport_NewBundleBuilder(t *testing.T) {
logger := mocks.NewLogger(ctrl)
fs := afero.NewMemMapFs()
finch := fpath.Finch("mockfinch")
lima := mocks.NewMockLimaWrapper(ctrl)

config := NewBundleConfig(finch, "mockhome")
NewBundleBuilder(logger, fs, config, finch, ecc)
NewBundleBuilder(logger, fs, config, finch, ecc, lima)
}

func TestSupportBundleBuilder_GenerateSupportBundle(t *testing.T) {
t.Parallel()

mockUser := &user.User{
Username: "mockuser",
}

testCases := []struct {
name string
mockSvc func(*mocks.Logger, *mocks.BundleConfig, *mocks.CommandCreator, *mocks.Command)
mockSvc func(*mocks.Logger, *mocks.BundleConfig, *mocks.CommandCreator, *mocks.Command, *mocks.MockLimaWrapper)
include []string
exclude []string
}{
{
name: "Generate support bundle",
mockSvc: func(logger *mocks.Logger, config *mocks.BundleConfig, ecc *mocks.CommandCreator, cmd *mocks.Command) {
mockSvc: func(
logger *mocks.Logger,
config *mocks.BundleConfig,
ecc *mocks.CommandCreator,
cmd *mocks.Command,
lima *mocks.MockLimaWrapper,
) {
logger.EXPECT().Debugf("Creating %s...", gomock.Any())
logger.EXPECT().Debugln("Gathering platform data...")

Expand All @@ -67,13 +79,21 @@ func TestSupportBundleBuilder_GenerateSupportBundle(t *testing.T) {
logger.EXPECT().Debugf("Copying %s...", "config1")
logger.EXPECT().Debugf("Copying %s...", "config2")
logger.EXPECT().Debugln("Copying in additional files...")

lima.EXPECT().LimaUser(false).Return(mockUser, nil).AnyTimes()
},
include: []string{},
exclude: []string{},
},
{
name: "Generate support bundle with an extra file included",
mockSvc: func(logger *mocks.Logger, config *mocks.BundleConfig, ecc *mocks.CommandCreator, cmd *mocks.Command) {
mockSvc: func(
logger *mocks.Logger,
config *mocks.BundleConfig,
ecc *mocks.CommandCreator,
cmd *mocks.Command,
lima *mocks.MockLimaWrapper,
) {
logger.EXPECT().Debugf("Creating %s...", gomock.Any())
logger.EXPECT().Debugln("Gathering platform data...")

Expand All @@ -96,13 +116,21 @@ func TestSupportBundleBuilder_GenerateSupportBundle(t *testing.T) {
logger.EXPECT().Debugf("Copying %s...", "config1")
logger.EXPECT().Debugln("Copying in additional files...")
logger.EXPECT().Debugf("Copying %s...", "extra1")

lima.EXPECT().LimaUser(false).Return(mockUser, nil).AnyTimes()
},
include: []string{"extra1"},
exclude: []string{},
},
{
name: "Generate support bundle with a log file excluded",
mockSvc: func(logger *mocks.Logger, config *mocks.BundleConfig, ecc *mocks.CommandCreator, cmd *mocks.Command) {
mockSvc: func(
logger *mocks.Logger,
config *mocks.BundleConfig,
ecc *mocks.CommandCreator,
cmd *mocks.Command,
lima *mocks.MockLimaWrapper,
) {
logger.EXPECT().Debugf("Creating %s...", gomock.Any())
logger.EXPECT().Debugln("Gathering platform data...")

Expand All @@ -124,13 +152,21 @@ func TestSupportBundleBuilder_GenerateSupportBundle(t *testing.T) {
logger.EXPECT().Debugln("Copying in config files...")
logger.EXPECT().Debugf("Copying %s...", "config1")
logger.EXPECT().Debugln("Copying in additional files...")

lima.EXPECT().LimaUser(false).Return(mockUser, nil).AnyTimes()
},
include: []string{},
exclude: []string{"log1"},
},
{
name: "Generate support bundle with a config file excluded",
mockSvc: func(logger *mocks.Logger, config *mocks.BundleConfig, ecc *mocks.CommandCreator, cmd *mocks.Command) {
mockSvc: func(
logger *mocks.Logger,
config *mocks.BundleConfig,
ecc *mocks.CommandCreator,
cmd *mocks.Command,
lima *mocks.MockLimaWrapper,
) {
logger.EXPECT().Debugf("Creating %s...", gomock.Any())
logger.EXPECT().Debugln("Gathering platform data...")

Expand All @@ -152,13 +188,21 @@ func TestSupportBundleBuilder_GenerateSupportBundle(t *testing.T) {
logger.EXPECT().Debugln("Copying in config files...")
logger.EXPECT().Infof("Excluding %s...", "config1")
logger.EXPECT().Debugln("Copying in additional files...")

lima.EXPECT().LimaUser(false).Return(mockUser, nil).AnyTimes()
},
include: []string{},
exclude: []string{"config1"},
},
{
name: "Generate support bundle with an included file excluded",
mockSvc: func(logger *mocks.Logger, config *mocks.BundleConfig, ecc *mocks.CommandCreator, cmd *mocks.Command) {
mockSvc: func(
logger *mocks.Logger,
config *mocks.BundleConfig,
ecc *mocks.CommandCreator,
cmd *mocks.Command,
lima *mocks.MockLimaWrapper,
) {
logger.EXPECT().Debugf("Creating %s...", gomock.Any())
logger.EXPECT().Debugln("Gathering platform data...")

Expand All @@ -181,6 +225,8 @@ func TestSupportBundleBuilder_GenerateSupportBundle(t *testing.T) {
logger.EXPECT().Debugf("Copying %s...", "config1")
logger.EXPECT().Debugln("Copying in additional files...")
logger.EXPECT().Infof("Excluding %s...", "extra1")

lima.EXPECT().LimaUser(false).Return(mockUser, nil).AnyTimes()
},
include: []string{"extra1"},
exclude: []string{"extra1"},
Expand All @@ -198,6 +244,7 @@ func TestSupportBundleBuilder_GenerateSupportBundle(t *testing.T) {
config := mocks.NewBundleConfig(ctrl)
finch := fpath.Finch("mockfinch")
ecc := mocks.NewCommandCreator(ctrl)
lima := mocks.NewMockLimaWrapper(ctrl)
cmd := mocks.NewCommand(ctrl)

builder := &bundleBuilder{
Expand All @@ -206,6 +253,7 @@ func TestSupportBundleBuilder_GenerateSupportBundle(t *testing.T) {
config: config,
finch: finch,
ecc: ecc,
lima: lima,
}

testFiles := []string{
Expand All @@ -225,7 +273,7 @@ func TestSupportBundleBuilder_GenerateSupportBundle(t *testing.T) {
require.NoError(t, err)
}

tc.mockSvc(logger, config, ecc, cmd)
tc.mockSvc(logger, config, ecc, cmd, lima)

zipFile, err := builder.GenerateSupportBundle(tc.include, tc.exclude)
assert.NoError(t, err)
Expand Down