From 1e2cf39faac5cf2496ed7749ec2cd808020aa201 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Mon, 27 Jun 2022 18:12:46 -0700 Subject: [PATCH] Update target creation to force fatal on error --- internal/core/machine_test.go | 47 +++++++++---------- internal/core/project_test.go | 15 ++---- internal/core/target_test.go | 8 ++-- internal/core/testing_target.go | 82 ++++++++++++++++++--------------- 4 files changed, 74 insertions(+), 78 deletions(-) diff --git a/internal/core/machine_test.go b/internal/core/machine_test.go index a532f299c..165a75c64 100644 --- a/internal/core/machine_test.go +++ b/internal/core/machine_test.go @@ -10,15 +10,10 @@ import ( "github.com/hashicorp/vagrant/internal/server/proto/vagrant_server" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" - "google.golang.org/protobuf/types/known/anypb" - "google.golang.org/protobuf/types/known/wrapperspb" ) func TestMachineSetValidId(t *testing.T) { - tm, err := TestMinimalMachine(t) - if err != nil { - t.Fatal(err) - } + tm := TestMinimalMachine(t) // Set valid id tm.SetID("something") @@ -41,7 +36,7 @@ func TestMachineSetValidId(t *testing.T) { } func TestMachineSetEmptyId(t *testing.T) { - tm, _ := TestMinimalMachine(t) + tm := TestMinimalMachine(t) oldId := tm.target.ResourceId // Set empty id @@ -85,7 +80,7 @@ func TestMachineSetEmptyId(t *testing.T) { } func TestMachineSetIdBlankThenSomethingPreservesDataDir(t *testing.T) { - tm, _ := TestMinimalMachine(t) + tm := TestMinimalMachine(t) // Set empty id, followed by a temp id. This is the same thing that happens // in the Docker provider's InitState action @@ -102,8 +97,8 @@ func TestMachineSetIdBlankThenSomethingPreservesDataDir(t *testing.T) { func TestMachineGetNonExistentBox(t *testing.T) { tp := TestMinimalProject(t) - tm, _ := TestMachine(t, tp, - WithTestTargetConfig(testBoxConfig("somename")), + tm := TestMachine(t, tp, + WithTestTargetConfig(testBoxConfig("somebox")), WithTestTargetProvider("testprovider"), ) @@ -144,7 +139,7 @@ func TestMachineGetExistentBox(t *testing.T) { func TestMachineConfigedGuest(t *testing.T) { type test struct { - config *vagrant_plugin_sdk.Args_ConfigData + config *component.ConfigData errors bool } @@ -153,7 +148,7 @@ func TestMachineConfigedGuest(t *testing.T) { {config: testGuestConfig("idontexist"), errors: true}, } guestMock := BuildTestGuestPlugin("myguest", "") - guestMock.On("Detect", mock.AnythingOfType("*core.Machine")).Return(false, nil) + guestMock.On("Detect", mock.AnythingOfType("*core.Machine")).Return(true, nil) guestMock.On("Parent").Return("", nil) pluginManager := plugin.TestManager(t, @@ -166,7 +161,7 @@ func TestMachineConfigedGuest(t *testing.T) { for _, tc := range tests { tp := TestProject(t, WithPluginManager(pluginManager)) - tm, _ := TestMachine(t, tp, + tm := TestMachine(t, tp, WithTestTargetConfig(tc.config), ) guest, err := tm.Guest() @@ -228,7 +223,7 @@ func TestMachineNoConfigGuest(t *testing.T) { pluginManager := plugin.TestManager(t, tc.plugins...) tp := TestProject(t, WithPluginManager(pluginManager)) - tm, _ := TestMachine(t, tp, WithTestTargetMinimalConfig()) + tm := TestMachine(t, tp) guest, err := tm.Guest() if tc.errors { require.Error(t, err) @@ -247,7 +242,7 @@ func TestMachineNoConfigGuest(t *testing.T) { } func TestMachineSetState(t *testing.T) { - tm, _ := TestMinimalMachine(t) + tm := TestMinimalMachine(t) type test struct { id string @@ -297,7 +292,7 @@ func TestMachineSyncedFolders(t *testing.T) { errors: false, config: testSyncedFolderConfig( []*testSyncedFolder{ - &testSyncedFolder{ + { source: ".", destination: "/vagrant", kind: "mysyncedfolder", @@ -312,20 +307,20 @@ func TestMachineSyncedFolders(t *testing.T) { errors: false, config: testSyncedFolderConfig( []*testSyncedFolder{ - &testSyncedFolder{ + { source: ".", destination: "/vagrant", kind: "mysyncedfolder", }, - &testSyncedFolder{ + { source: "./two", destination: "/vagrant-two", kind: "mysyncedfolder", }, - &testSyncedFolder{ + { source: "./three", destination: "/vagrant-three", - kind: "mysyncedfolder", + kind: "myothersyncedfolder", }, }, ), @@ -337,20 +332,20 @@ func TestMachineSyncedFolders(t *testing.T) { errors: true, config: testSyncedFolderConfig( []*testSyncedFolder{ - &testSyncedFolder{ + { source: ".", destination: "/vagrant", - kind: "mysyncedfolder", + kind: "idontexist", }, - &testSyncedFolder{ + { source: "./two", destination: "/vagrant-two", kind: "mysyncedfolder", }, - &testSyncedFolder{ + { source: "./three", destination: "/vagrant-three", - kind: "mysyncedfolder", + kind: "myothersyncedfolder", }, }, ), @@ -360,7 +355,7 @@ func TestMachineSyncedFolders(t *testing.T) { for _, tc := range tests { pluginManager := plugin.TestManager(t, tc.plugins...) tp := TestProject(t, WithPluginManager(pluginManager)) - tm, _ := TestMachine(t, tp, + tm := TestMachine(t, tp, WithTestTargetConfig(tc.config), ) folders, err := tm.SyncedFolders() diff --git a/internal/core/project_test.go b/internal/core/project_test.go index ddc444674..514443b00 100644 --- a/internal/core/project_test.go +++ b/internal/core/project_test.go @@ -11,14 +11,11 @@ import ( func projectTargets(t *testing.T, project *Project, numTargets int) (targets []*Target) { targets = make([]*Target, numTargets) for i := 0; i < numTargets; i++ { - tt, err := TestTarget(t, project, &vagrant_server.Target{ + tt := TestTarget(t, project, &vagrant_server.Target{ ResourceId: fmt.Sprintf("id-%d", i), Name: fmt.Sprintf("target-%d", i), Uuid: fmt.Sprintf("uuid-%d", i), }) - if err != nil { - t.Error(err) - } targets = append(targets, tt) } return @@ -35,14 +32,8 @@ func TestNewProject(t *testing.T) { func TestProjectGetTarget(t *testing.T) { tp := TestMinimalProject(t) // Add targets to project - targetOne, err := TestTarget(t, tp, &vagrant_server.Target{ResourceId: "id-one", Name: "target-one"}) - if err != nil { - t.Error(err) - } - targetTwo, err := TestTarget(t, tp, &vagrant_server.Target{ResourceId: "id-two", Name: "target-two"}) - if err != nil { - t.Error(err) - } + targetOne := TestTarget(t, tp, &vagrant_server.Target{ResourceId: "id-one", Name: "target-one"}) + targetTwo := TestTarget(t, tp, &vagrant_server.Target{ResourceId: "id-two", Name: "target-two"}) // Get by id one, err := tp.Target("id-one", "") diff --git a/internal/core/target_test.go b/internal/core/target_test.go index 5db939c94..e67b838a4 100644 --- a/internal/core/target_test.go +++ b/internal/core/target_test.go @@ -9,7 +9,7 @@ import ( ) func TestTargetSpecializeMachine(t *testing.T) { - tt, _ := TestMinimalTarget(t) + tt := TestMinimalTarget(t) specialized, err := tt.Specialize((*core.Machine)(nil)) if err != nil { t.Errorf("Specialize function returned an error") @@ -28,8 +28,8 @@ func TestTargetSpecializeMachine(t *testing.T) { func TestTargetSpecializeMultiMachine(t *testing.T) { p := TestMinimalProject(t) - tt1, _ := TestTarget(t, p, &vagrant_server.Target{Name: "tt1"}) - tt2, _ := TestTarget(t, p, &vagrant_server.Target{Name: "tt2"}) + tt1 := TestTarget(t, p, &vagrant_server.Target{Name: "tt1"}) + tt2 := TestTarget(t, p, &vagrant_server.Target{Name: "tt2"}) specialized, err := tt1.Specialize((*core.Machine)(nil)) if err != nil { @@ -53,7 +53,7 @@ func TestTargetSpecializeMultiMachine(t *testing.T) { } func TestTargetSpecializeBad(t *testing.T) { - tt, _ := TestMinimalTarget(t) + tt := TestMinimalTarget(t) specialized, err := tt.Specialize((*core.Project)(nil)) if err != nil { diff --git a/internal/core/testing_target.go b/internal/core/testing_target.go index d3f5de23c..5e32e7c3c 100644 --- a/internal/core/testing_target.go +++ b/internal/core/testing_target.go @@ -3,7 +3,10 @@ package core import ( "context" + "github.com/imdario/mergo" + "github.com/hashicorp/go-multierror" + "github.com/hashicorp/vagrant-plugin-sdk/component" "github.com/hashicorp/vagrant-plugin-sdk/core" "github.com/hashicorp/vagrant-plugin-sdk/proto/vagrant_plugin_sdk" "github.com/hashicorp/vagrant/internal/server/proto/vagrant_server" @@ -14,32 +17,44 @@ import ( // TestTarget returns a fully in-memory and side-effect free Target that // can be used for testing. Additional options can be given to provide your own // factories, configuration, etc. -func TestTarget(t testing.T, tp *Project, tt *vagrant_server.Target) (target *Target, err error) { - testingTarget := ptypes.TestTarget(t, tt) - testingTarget.Project = tp.Ref().(*vagrant_plugin_sdk.Ref_Project) - tp.basis.client.UpsertTarget( +func TestTarget(t testing.T, p *Project, st *vagrant_server.Target) (target *Target) { + testingTarget := ptypes.TestTarget(t, st) + testingTarget.Project = p.Ref().(*vagrant_plugin_sdk.Ref_Project) + _, err := p.basis.client.UpsertTarget( context.Background(), &vagrant_server.UpsertTargetRequest{ - Project: tp.Ref().(*vagrant_plugin_sdk.Ref_Project), + Project: p.Ref().(*vagrant_plugin_sdk.Ref_Project), Target: testingTarget, }, ) - target, err = tp.LoadTarget([]TargetOption{ - WithTargetRef(&vagrant_plugin_sdk.Ref_Target{Project: tp.Ref().(*vagrant_plugin_sdk.Ref_Project), Name: testingTarget.Name}), + if err != nil { + t.Fatal(err) + } + + target, err = p.LoadTarget([]TargetOption{ + WithTargetRef( + &vagrant_plugin_sdk.Ref_Target{ + Project: p.Ref().(*vagrant_plugin_sdk.Ref_Project), + Name: testingTarget.Name, + }, + ), }...) if err != nil { t.Fatal(err) - return } - tp.project.Targets = append(tp.project.Targets, target.Ref().(*vagrant_plugin_sdk.Ref_Target)) + + if err = p.refreshProject(); err != nil { + t.Fatal(err) + } + return } // TestMinimalTarget uses a minimal project to setup the most basic target // that will work for testing -func TestMinimalTarget(t testing.T) (target *Target, err error) { +func TestMinimalTarget(t testing.T) (target *Target) { tp := TestMinimalProject(t) - tp.basis.client.UpsertTarget( + _, err := tp.basis.client.UpsertTarget( context.Background(), &vagrant_server.UpsertTargetRequest{ Project: tp.Ref().(*vagrant_plugin_sdk.Ref_Project), @@ -49,10 +64,18 @@ func TestMinimalTarget(t testing.T) (target *Target, err error) { }, }, ) - target, err = tp.LoadTarget([]TargetOption{ - WithTargetRef(&vagrant_plugin_sdk.Ref_Target{Project: tp.Ref().(*vagrant_plugin_sdk.Ref_Project), Name: "test-target"}), - }...) + if err != nil { + t.Fatal(err) + } + target, err = tp.LoadTarget([]TargetOption{ + WithTargetRef( + &vagrant_plugin_sdk.Ref_Target{ + Project: tp.Ref().(*vagrant_plugin_sdk.Ref_Project), + Name: "test-target", + }, + ), + }...) if err != nil { t.Fatal(err) } @@ -63,57 +86,44 @@ func TestMinimalTarget(t testing.T) (target *Target, err error) { // TestMachine returns a fully in-memory and side-effect free Machine that // can be used for testing. Additional options can be given to provide your own // factories, configuration, etc. -func TestMachine(t testing.T, tp *Project, opts ...TestMachineOption) (machine *Machine, err error) { - tt, _ := TestTarget(t, tp, &vagrant_server.Target{}) +func TestMachine(t testing.T, tp *Project, opts ...TestMachineOption) (machine *Machine) { + tt := TestTarget(t, tp, &vagrant_server.Target{}) specialized, err := tt.Specialize((*core.Machine)(nil)) if err != nil { - return nil, err + t.Fatal(err) } + machine = specialized.(*Machine) for _, opt := range opts { if oerr := opt(machine); oerr != nil { err = multierror.Append(err, oerr) } } - if err != nil { t.Fatal(err) } + return } // TestMinimalMachine uses a minimal project to setup the most basic machine // that will work for testing -func TestMinimalMachine(t testing.T) (machine *Machine, err error) { +func TestMinimalMachine(t testing.T) (machine *Machine) { tp := TestMinimalProject(t) - tt, err := TestTarget(t, tp, &vagrant_server.Target{}) - if err != nil { - t.Fatal(err) - return - } + tt := TestTarget(t, tp, &vagrant_server.Target{}) specialized, err := tt.Specialize((*core.Machine)(nil)) if err != nil { t.Fatal(err) - return nil, err } machine = specialized.(*Machine) - WithTestTargetMinimalConfig()(machine) return } type TestMachineOption func(*Machine) error -func WithTestTargetMinimalConfig() TestMachineOption { +func WithTestTargetConfig(config *component.ConfigData) TestMachineOption { return func(m *Machine) (err error) { - m.target.Configuration = &vagrant_plugin_sdk.Args_ConfigData{} - return - } -} - -func WithTestTargetConfig(config *vagrant_plugin_sdk.Args_ConfigData) TestMachineOption { - return func(m *Machine) (err error) { - m.target.Configuration = config - return + return mergo.Merge(m.vagrantfile.root, config) } }