Skip to content

Commit

Permalink
chore: split e2e tests into 2 packages, vm and container (#150)
Browse files Browse the repository at this point in the history
## Summary

PR fixes
#142 (comment) by
splitting container tests and vm tests into different binaries (i.e.,
different packages) to make sure that the former is run before the
latter.

The package-level comment of the `e2e` package provides more details, so
reading it first may make reviewing this PR easier.

## License Acceptance

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.

Signed-off-by: Hsing-Yu (David) Chen <[email protected]>
  • Loading branch information
davidhsingyuchen authored Jan 12, 2023
1 parent bfdaf56 commit 8cacb1f
Show file tree
Hide file tree
Showing 10 changed files with 133 additions and 48 deletions.
6 changes: 5 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -243,9 +243,13 @@ test-unit:
go test $(shell go list ./... | grep -v e2e) -shuffle on -race

# test-e2e assumes the VM instance doesn't exist, please make sure to remove it before running.
#
# Container tests have to be run before VM tests according to the current code setup.
# For more details, see the package-level comment of the e2e package.
.PHONY: test-e2e
test-e2e:
go test -ldflags $(LDFLAGS) -timeout 60m ./e2e/... -test.v -ginkgo.v --installed="$(INSTALLED)"
go test -ldflags $(LDFLAGS) -timeout 30m ./e2e/container -test.v -ginkgo.v --installed="$(INSTALLED)"
go test -ldflags $(LDFLAGS) -timeout 30m ./e2e/vm -test.v -ginkgo.v --installed="$(INSTALLED)"

.PHONY: gen-code
# Since different projects may have different versions of tool binaries,
Expand Down
45 changes: 9 additions & 36 deletions e2e/e2e_test.go → e2e/container/container_test.go
Original file line number Diff line number Diff line change
@@ -1,57 +1,36 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

package e2e
// Package container runs tests related to container development.
package container

import (
"flag"
"os"
"path/filepath"
"testing"

"github.com/onsi/ginkgo/v2"
"github.com/onsi/gomega"
"github.com/runfinch/common-tests/command"
"github.com/runfinch/common-tests/option"
"github.com/runfinch/common-tests/tests"
)

const (
virtualMachineRootCmd = "vm"
installedTestSubject = "finch"
"github.com/runfinch/finch/e2e"
)

var installed = flag.Bool("installed", false, "the flag to show whether the tests are ran against installed application")

//nolint:paralleltest // TestE2e is like TestMain for our e2e tests.
func TestE2e(t *testing.T) {
description := "Finch CLI e2e Tests"
//nolint:paralleltest // TestContainer is like TestMain for the container-related tests.
func TestContainer(t *testing.T) {
const description = "Finch Container Development E2E Tests"

wd, err := os.Getwd()
o, err := e2e.CreateOption()
if err != nil {
t.Fatalf("failed to get the current working directory: %v", err)
}
subject := filepath.Join(wd, "../_output/bin/finch")
if *installed {
subject = installedTestSubject
}

o, err := option.New([]string{subject})
if err != nil {
t.Fatalf("failed to initialize a testing option: %v", err)
t.Fatal(err)
}

// The VM should be cleaned up in SynchronizedAfterSuite of e2e/vm.
ginkgo.SynchronizedBeforeSuite(func() []byte {
command.New(o, "vm", "init").WithTimeoutInSeconds(600).Run()
tests.SetupLocalRegistry(o)
return nil
}, func(bytes []byte) {})

ginkgo.SynchronizedAfterSuite(func() {
command.New(o, "vm", "stop").WithTimeoutInSeconds(60).Run()
command.New(o, "vm", "remove").WithTimeoutInSeconds(60).Run()
}, func() {})

ginkgo.Describe(description, func() {
tests.Pull(o)
tests.Rm(o)
Expand Down Expand Up @@ -96,12 +75,6 @@ func TestE2e(t *testing.T) {
tests.NetworkInspect(o)
tests.NetworkLs(o)
tests.NetworkRm(o)
// When running tests in serial sequence and using the local registry, testVirtualMachine needs to run after generic tests finished
// since it will remove the VM instance thus removing the local registry.
testVirtualMachine(o)
testAdditionalDisk(o)
testConfig(o, *installed)
testVersion(o)
})

gomega.RegisterFailHandler(ginkgo.Fail)
Expand Down
61 changes: 61 additions & 0 deletions e2e/e2e.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

// Package e2e is created because:
//
// During test setup, SetupLocalRegistry is called,
// and container tests need to be run before VM tests
// so that the local registry won't be removed by VM tests,
// and container tests can use the images stored in it.
//
// However, by default Ginkgo does not guarantee the order in which specs run [1],
// and Ginkgo doc says that "You should only ever call RunSpecs once" [2],
// which means that we need two binaries that run VM tests and container tests respectively.
//
// We could add ginkgo.Ordered to the top level Ginkgo container,
// but that will make every single spec to run in the order they are defined:
// "Any container nodes nested within a container node will automatically be considered Ordered
// and there is no way to mark a node within an Ordered container as "not Ordered"" [1],
// and we don't want that because it can hide implicit dependencies among tests,
// while each test should be independent unless specially designed.
//
// As a result, we split the tests into `e2e/vm` and `e2e/container` packages
// and extract the common logic into this package.
//
// [1] https://onsi.github.io/ginkgo/#ordered-containers.
// [2] https://onsi.github.io/ginkgo/#mental-model-go-modules-packages-and-tests
package e2e

import (
"flag"
"fmt"
"os"
"path/filepath"

"github.com/runfinch/common-tests/option"
)

// InstalledTestSubject is the test subject when Finch is installed.
const InstalledTestSubject = "finch"

// Installed indicates whether the tests are run against installed application.
var Installed = flag.Bool("installed", false, "the flag to show whether the tests are run against installed application")

// CreateOption creates an option for running e2e tests.
func CreateOption() (*option.Option, error) {
wd, err := os.Getwd()
if err != nil {
return nil, fmt.Errorf("failed to get the current working directory: %w", err)
}

subject := filepath.Join(wd, "../../_output/bin/finch")
if *Installed {
subject = InstalledTestSubject
}

o, err := option.New([]string{subject})
if err != nil {
return nil, fmt.Errorf("failed to initialize a testing option: %w", err)
}
return o, nil
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

package e2e
package vm

import (
"github.com/onsi/ginkgo/v2"
Expand Down
10 changes: 6 additions & 4 deletions e2e/config_test.go → e2e/vm/config_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

package e2e
package vm

import (
"errors"
Expand All @@ -18,11 +18,13 @@ import (
"github.com/runfinch/common-tests/option"
"github.com/xorcare/pointer"
"gopkg.in/yaml.v3"

"github.com/runfinch/finch/e2e"
)

var finchConfigFilePath = os.Getenv("HOME") + "/.finch/finch.yaml"

const defaultLimaConfigFilePath = "../_output/lima/data/_config/override.yaml"
const defaultLimaConfigFilePath = "../../_output/lima/data/_config/override.yaml"

func readFile(filePath string) []byte {
out, err := os.ReadFile(filepath.Clean(filePath))
Expand Down Expand Up @@ -65,11 +67,11 @@ var testConfig = func(o *option.Option, installed bool) {
origFinchCfg := readFile(finchConfigFilePath)
limaConfigFilePath = defaultLimaConfigFilePath
if installed {
path, err := exec.LookPath(installedTestSubject)
path, err := exec.LookPath(e2e.InstalledTestSubject)
gomega.Expect(err).ShouldNot(gomega.HaveOccurred())
realFinchPath, err := filepath.EvalSymlinks(path)
gomega.Expect(err).ShouldNot(gomega.HaveOccurred())
limaConfigFilePath = filepath.Join(realFinchPath, "/../../lima/data/_config/override.yaml")
limaConfigFilePath = filepath.Join(realFinchPath, "../../lima/data/_config/override.yaml")
}
origLimaCfg := readFile(limaConfigFilePath)

Expand Down
4 changes: 2 additions & 2 deletions e2e/virtual_machine_test.go → e2e/vm/lifecycle_test.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

package e2e
package vm

import (
"github.com/onsi/ginkgo/v2"
"github.com/runfinch/common-tests/command"
"github.com/runfinch/common-tests/option"
)

var testVirtualMachine = func(o *option.Option) {
var testVMLifecycle = func(o *option.Option) {
// These tests are run in serial because we only define one virtual machine instance.
ginkgo.Describe("virtual machine lifecycle", ginkgo.Serial, func() {
ginkgo.When("the virtual machine is in running status", func() {
Expand Down
2 changes: 1 addition & 1 deletion e2e/version_test.go → e2e/vm/version_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

package e2e
package vm

import (
"fmt"
Expand Down
45 changes: 45 additions & 0 deletions e2e/vm/vm_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

// Package vm runs tests related to the virtual machine.
package vm

import (
"testing"

"github.com/onsi/ginkgo/v2"
"github.com/onsi/gomega"
"github.com/runfinch/common-tests/command"

"github.com/runfinch/finch/e2e"
)

const (
virtualMachineRootCmd = "vm"
)

//nolint:paralleltest // TestVM is like TestMain for the VM-related tests.
func TestVM(t *testing.T) {
const description = "Finch Virtual Machine E2E Tests"

o, err := e2e.CreateOption()
if err != nil {
t.Fatal(err)
}

// The VM should be spined up in SynchronizedBeforeSuite of e2e/container.
ginkgo.SynchronizedAfterSuite(func() {
command.New(o, "vm", "stop").WithTimeoutInSeconds(60).Run()
command.New(o, "vm", "remove").WithTimeoutInSeconds(60).Run()
}, func() {})

ginkgo.Describe("", func() {
testVMLifecycle(o)
testAdditionalDisk(o)
testConfig(o, *e2e.Installed)
testVersion(o)
})

gomega.RegisterFailHandler(ginkgo.Fail)
ginkgo.RunSpecs(t, description)
}
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ require (
github.com/golang/mock v1.6.0
github.com/google/go-licenses v1.5.0
github.com/lima-vm/lima v0.14.2
github.com/onsi/ginkgo/v2 v2.6.1
github.com/onsi/ginkgo/v2 v2.7.0
github.com/onsi/gomega v1.24.2
github.com/pelletier/go-toml v1.9.5
github.com/pkg/sftp v1.13.5
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -443,8 +443,8 @@ github.com/onsi/ginkgo v1.16.5/go.mod h1:+E8gABHa3K6zRBolWtd+ROzc/U5bkGt0FwiG042
github.com/onsi/ginkgo/v2 v2.1.3/go.mod h1:vw5CSIxN1JObi/U8gcbwft7ZxR2dgaR70JSE3/PpL4c=
github.com/onsi/ginkgo/v2 v2.1.4/go.mod h1:um6tUpWM/cxCK3/FK8BXqEiUMUwRgSM4JXG47RKZmLU=
github.com/onsi/ginkgo/v2 v2.1.6/go.mod h1:MEH45j8TBi6u9BMogfbp0stKC5cdGjumZj5Y7AG4VIk=
github.com/onsi/ginkgo/v2 v2.6.1 h1:1xQPCjcqYw/J5LchOcp4/2q/jzJFjiAOc25chhnDw+Q=
github.com/onsi/ginkgo/v2 v2.6.1/go.mod h1:yjiuMwPokqY1XauOgju45q3sJt6VzQ/Fict1LFVcsAo=
github.com/onsi/ginkgo/v2 v2.7.0 h1:/XxtEV3I3Eif/HobnVx9YmJgk8ENdRsuUmM+fLCFNow=
github.com/onsi/ginkgo/v2 v2.7.0/go.mod h1:yjiuMwPokqY1XauOgju45q3sJt6VzQ/Fict1LFVcsAo=
github.com/onsi/gomega v0.0.0-20170829124025-dcabb60a477c/go.mod h1:C1qb7wdrVGGVU+Z6iS04AVkA3Q65CEZX59MT0QO5uiA=
github.com/onsi/gomega v1.5.0/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY=
github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7JYyY=
Expand Down

0 comments on commit 8cacb1f

Please sign in to comment.