diff --git a/.circleci/config.yml b/.circleci/config.yml index 9b9440e22..cfa817c39 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -520,10 +520,6 @@ workflows: - /^backport/docs-.*/ - stable-website - # Note: comment-out this job in ENT - - build-darwin-binaries: - filters: *backend_check_branches_filter - - test-e2e: filters: *backend_check_branches_filter @@ -546,51 +542,3 @@ workflows: - /^backport/docs-.*/ - /^e2e-.*/ - stable-website - - - test-machine: - name: "test-client" - test_packages: "./client/..." - # test branches are the branches that can impact unit tests - filters: &backend_test_branches_filter - branches: - ignore: - - /^.-ui\b.*/ - - /^docs-.*/ - - /^backport/docs-.*/ - - /^e2e-.*/ - - stable-website - - test-machine: - name: "test-nomad" - test_packages: "./nomad/..." - filters: *backend_test_branches_filter - - test-machine: - # API Tests run in a VM rather than container due to the FS tests - # requiring `mount` priviliges. - name: "test-api" - test_module: "api" - filters: *backend_test_branches_filter - enable_race_testing: true - - test-machine: - name: "test-other" - exclude_packages: "./api|./client|./drivers/docker|./drivers/exec|./drivers/shared/executor|./nomad|./e2e" - filters: *backend_test_branches_filter - - test-machine: - name: "test-docker" - test_packages: "./drivers/docker" - executor: go-machine - filters: *backend_test_branches_filter - - test-machine: - name: "test-exec" - test_packages: "./drivers/exec" - filters: *backend_test_branches_filter - - test-machine: - name: "test-shared-exec" - test_packages: "./drivers/shared/executor" - filters: *backend_test_branches_filter - - test-machine: - name: "test-32bit" - # Currently we only explicitly test fingerprinting on 32bit - # architectures. - test_packages: "./client/fingerprint" - goarch: "386" - filters: *backend_test_branches_filter diff --git a/.github/workflows/test-core.yaml b/.github/workflows/test-core.yaml index b1c21f799..2dad62109 100644 --- a/.github/workflows/test-core.yaml +++ b/.github/workflows/test-core.yaml @@ -25,8 +25,8 @@ env: GO_VERSION: 1.19.1 GOBIN: /usr/local/bin GOTESTARCH: amd64 - CONSUL_VERSION: 1.11.3 - VAULT_VERSION: 1.9.3 + CONSUL_VERSION: 1.12.6 + VAULT_VERSION: 1.12.0 NOMAD_SLOW_TEST: 0 NOMAD_TEST_LOG_LEVEL: OFF jobs: @@ -48,7 +48,7 @@ jobs: runs-on: ubuntu-22.04 timeout-minutes: 10 steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 with: fetch-depth: 0 # needs tags for checkproto - uses: magnetikonline/action-golang-cache@v1 @@ -61,7 +61,7 @@ jobs: make bootstrap make check compile: - needs: [mods] + needs: [mods, checks] strategy: fail-fast: false matrix: @@ -69,7 +69,7 @@ jobs: runs-on: ${{matrix.os}} timeout-minutes: 20 steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 - uses: magnetikonline/action-golang-cache@v1 with: go-version: ${{env.GO_VERSION}} @@ -85,7 +85,7 @@ jobs: runs-on: ubuntu-22.04 timeout-minutes: 30 steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 - uses: magnetikonline/action-golang-cache@v1 with: go-version: ${{env.GO_VERSION}} @@ -98,72 +98,34 @@ jobs: make generate-all sudo sed -i 's!Defaults!#Defaults!g' /etc/sudoers sudo -E env "PATH=$PATH" make test-nomad-module - tests-pkgs: + tests-groups: needs: [mods] runs-on: ubuntu-22.04 timeout-minutes: 30 strategy: fail-fast: false matrix: - pkg: - - acl/... - - client - - client/allocdir/... - - client/allochealth/... - - client/allocrunner/... - - client/allocwatcher/... - - client/config/... - - client/consul/... - - client/devicemanager/... - - client/dynamicplugins/... - - client/fingerprint/... - - client/interfaces/... - - client/lib/... - - client/logmon/... - - client/pluginmanager/... - - client/servers/... - - client/serviceregistration/... - - client/state/... - - client/stats/... - - client/structs/... - - client/taskenv/... - - command - - command/agent/... - - command/raft_tools/... - - drivers/docker/... - - drivers/exec/... - - drivers/java/... - - drivers/mock/... - - drivers/rawexec/... - - drivers/shared/... - - drivers/qemu/... - - helper/... - - internal/... - - jobspec/... - - lib/... + groups: - nomad - - nomad/deploymentwatcher/... - - nomad/drainer/... - - nomad/state/... - - nomad/stream/... - - nomad/structs/... - - nomad/volumewatcher/... - - plugins/... - - scheduler/... - - testutil/... + - client + - command + - drivers + - quick steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 - uses: magnetikonline/action-golang-cache@v1 with: go-version: ${{env.GO_VERSION}} cache-key-suffix: -core - name: Run Matrix Tests env: - GOTEST_PKGS: ./${{matrix.pkg}} + GOTEST_GROUP: ${{matrix.groups}} run: | make bootstrap make generate-all + make dev sudo hc-install install -version ${{env.VAULT_VERSION}} -path /usr/local/bin vault sudo hc-install install -version ${{env.CONSUL_VERSION}} -path /usr/local/bin consul sudo sed -i 's!Defaults!#Defaults!g' /etc/sudoers sudo -E env "PATH=$PATH" make test-nomad + diff --git a/.gitignore b/.gitignore index 0b0d15fcb..2ccc3d724 100644 --- a/.gitignore +++ b/.gitignore @@ -130,3 +130,6 @@ e2e/remotetasks/input/ecs.vars # local terraform overrides *.auto.tfvars + +# Tools files +tools/missing/missing diff --git a/GNUmakefile b/GNUmakefile index d4f3094f9..fd98c00fe 100644 --- a/GNUmakefile +++ b/GNUmakefile @@ -32,12 +32,6 @@ ifndef NOMAD_NO_UI GO_TAGS := ui $(GO_TAGS) endif -ifeq ($(origin GOTEST_PKGS_EXCLUDE), undefined) -GOTEST_PKGS ?= "./..." -else -GOTEST_PKGS=$(shell go list ./... | sed 's/github.com\/hashicorp\/nomad/./' | egrep -v "^($(GOTEST_PKGS_EXCLUDE))(/.*)?$$") -endif - # tag corresponding to latest release we maintain backward compatibility with PROTO_COMPARE_TAG ?= v1.0.3$(if $(findstring ent,$(GO_TAGS)),+ent,) @@ -283,9 +277,11 @@ release: clean $(foreach t,$(ALL_TARGETS),pkg/$(t).zip) ## Build all release pac @tree --dirsfirst $(PROJECT_ROOT)/pkg .PHONY: test-nomad -test-nomad: dev ## Run Nomad unit tests - @echo "==> Running Nomad unit tests $(GOTEST_PKGS)" - gotestsum --format=testname --rerun-fails=3 --packages=$(GOTEST_PKGS) -- \ +test-nomad: GOTEST_PKGS=$(shell go run -modfile=tools/go.mod tools/missing/main.go ci/test-core.json $(GOTEST_GROUP)) +test-nomad: # dev ## Run Nomad unit tests + @echo "==> Running Nomad unit tests $(GOTEST_GROUP)" + @echo "==> with packages $(GOTEST_PKGS)" + gotestsum --format=testname --rerun-fails=3 --packages="$(GOTEST_PKGS)" -- \ -cover \ -timeout=20m \ -count=1 \ @@ -407,7 +403,7 @@ endif .PHONY: missing missing: ## Check for packages not being tested @echo "==> Checking for packages not being tested ..." - @go run -modfile tools/go.mod tools/missing/main.go .github/workflows/test-core.yaml + @go run -modfile tools/go.mod tools/missing/main.go ci/test-core.json .PHONY: ec2info ec2info: ## Generate AWS EC2 CPU specification table diff --git a/ci/README.md b/ci/README.md new file mode 100644 index 000000000..fd8ebc47b --- /dev/null +++ b/ci/README.md @@ -0,0 +1,34 @@ +# CI (unit testing) + +This README describes how the Core CI Tests Github Actions works, which provides +Nomad with continuous integration unit testing. + +## Steps + +1. When a branch is pushed, GHA triggers `.github/workflows/test-core.yaml`. + +2. The first job is `mods` which creates a pre-cache of Go modules. + - Only useful for the followup jobs on Linux runners + - Is keyed on `hash(go.sum)`, so a cache is re-used until deps are modified. + +3. The `checks`, `test-api`, `test-*` jobs are started. + - The checks job runs `make check` + - The test job runs groups of tests, see below + +3i. The check step also runs `make missing` + - Invokes `tools/missing` to scan `ci/test-cores.json` && nomad source. + - Fails the build if any packages in Nomad are not covered. + +4a. The `test-*` jobs are run. + - Configured as a matrix of "groups"; each group is a set of packages. + - The GHA invokes `test-nomad` with $GOTEST_GROUP for each group. + - The makefile uses `tools/missing` to translate the group into packages + - Package groups are configured in `ci/test-core.json` + +4b. The `test-api` job is run. + - Because `api` is a submodule, invokation of test command is special. + - The GHA invokes `test-nomad-module` with the name of the submodule. + +5. The `compile` jobs are run + - Waits on checks to complete first + - Runs on each of `linux`, `macos`, `windows` diff --git a/ci/test-core.json b/ci/test-core.json new file mode 100644 index 000000000..2614f7ea3 --- /dev/null +++ b/ci/test-core.json @@ -0,0 +1,45 @@ +{ + "nomad": ["nomad"], + "client": [ + "client", + "client/allocrunner/..." + ], + "command": ["command"], + "drivers": ["drivers/..."], + "quick": [ + "acl/...", + "client/allocdir/...", + "client/allochealth/...", + "client/allocwatcher/...", + "client/config/...", + "client/consul/...", + "client/devicemanager/...", + "client/dynamicplugins/...", + "client/fingerprint/...", + "client/interfaces/...", + "client/lib/...", + "client/logmon/...", + "client/pluginmanager/...", + "client/servers/...", + "client/serviceregistration/...", + "client/state/...", + "client/stats/...", + "client/structs/...", + "client/taskenv/...", + "command/agent/...", + "command/raft_tools/...", + "helper/...", + "internal/...", + "jobspec/...", + "lib/...", + "nomad/deploymentwatcher/...", + "nomad/drainer/...", + "nomad/state/...", + "nomad/stream/...", + "nomad/structs/...", + "nomad/volumewatcher/...", + "plugins/...", + "scheduler/...", + "testutil/..." + ] +} diff --git a/contributing/testing.md b/contributing/testing.md index 7df1533a7..1794955fd 100644 --- a/contributing/testing.md +++ b/contributing/testing.md @@ -6,22 +6,25 @@ demonstrating correct functionality. Each unit test should meet a few criteria: -- Use testify - - Prefer using require.* functions +- Use [github.com/shoenig/test](https://github.com/shoenig/test) + - Prefer using `must.*`` functions + - Use `test.*`` functions when cleanup must happen, etc + - Feel free to refactor testify tests; but consider separate commits / PRs - Undo any changes to the environment - - Set environment variables must be unset - - Scratch files/dirs must be removed (use t.TempDir) - - Consumed ports must be freed (e.g. TestServer.Cleanup, freeport.Return) + - Set environment variables must be unset (use `t.Setenv`) + - Scratch files/dirs must be removed (use `t.TempDir`) + - Consumed ports must be freed (e.g. `TestServer.Cleanup`, `freeport.Return`) - Able to run in parallel - - All package level Test* functions should start with ci.Parallel + - All package level `Test*` functions should start with `ci.Parallel` - Always use dynamic scratch dirs, files - - Always get ports from helpers (TestServer, TestClient, TestAgent, freeport.Get) + - Always get ports from helpers (`TestServer`, `TestClient`, `TestAgent`, `freeport.Get`) - Log control - - Logging must go through the testing.T (use helper/testlog.HCLogger) + - Logging must go through the `testing.T` (use `helper/testlog.HCLogger`) - Avoid excessive logging in test cases - prefer failure messages + - Annotate failures with `must.Sprint\f` post-scripts ## API tests @@ -29,3 +32,9 @@ Testing in the `api` package requires an already-built Nomad binary. If you're writing `api` tests, you'll need to build a Nomad binary (ex. with `make dev`) that includes any changes your API exercises. + + +# CI Plumbing + +See [ci/README.md] for details on how the [Core CI Tests](https://github.com/hashicorp/nomad/actions/workflows/test-core.yaml) +Github Action works. diff --git a/tools/go.mod b/tools/go.mod index 05d73bfa4..4e570d979 100644 --- a/tools/go.mod +++ b/tools/go.mod @@ -4,13 +4,11 @@ go 1.18 require ( github.com/aws/aws-sdk-go v1.37.26 - github.com/stretchr/testify v1.7.1 - gopkg.in/yaml.v2 v2.2.8 + github.com/hashicorp/go-set v0.1.6 + github.com/shoenig/test v0.4.0 ) require ( - github.com/davecgh/go-spew v1.1.0 // indirect + github.com/google/go-cmp v0.5.8 // indirect github.com/jmespath/go-jmespath v0.4.0 // indirect - github.com/pmezard/go-difflib v1.0.0 // indirect - gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c // indirect ) diff --git a/tools/go.sum b/tools/go.sum index 06ed6b40d..5433d3e56 100644 --- a/tools/go.sum +++ b/tools/go.sum @@ -2,6 +2,10 @@ github.com/aws/aws-sdk-go v1.37.26 h1:D9Qvyjlr6xFR0CspZ0imdASc5Y1WE/Sgyte4l+cUp4 github.com/aws/aws-sdk-go v1.37.26/go.mod h1:hcU610XS61/+aQV88ixoOzUoG7v3b31pl2zKMmprdro= github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/google/go-cmp v0.5.8 h1:e6P7q2lk1O+qJJb4BtCQXlK8vWEO8V1ZeuEdJNOqZyg= +github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= +github.com/hashicorp/go-set v0.1.6 h1:fj/JG5B97sAOd9OQN4GL880yCE384fz3asNDpGbxkPo= +github.com/hashicorp/go-set v0.1.6/go.mod h1:ELvMcE+1mRHYPVgTFSQiecObIwZRxY5Q11EhbtmM5KQ= github.com/jmespath/go-jmespath v0.4.0 h1:BEgLn5cpjn8UN1mAw4NjwDrS35OdebyEtFe+9YPoQUg= github.com/jmespath/go-jmespath v0.4.0/go.mod h1:T8mJZnbsbmF+m6zOOFylbeCJqk5+pHWvzYPziyZiYoo= github.com/jmespath/go-jmespath/internal/testify v1.5.1 h1:shLQSRRSCCPj3f2gpwzGwWFoC7ycTf1rcQZHOlsJ6N8= @@ -9,9 +13,9 @@ github.com/jmespath/go-jmespath/internal/testify v1.5.1/go.mod h1:L3OGu8Wl2/fWfC github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/shoenig/test v0.4.0 h1:3X4xG/Chx7mzi0h71Sm6Vo38q0EYaQIBZpYFRcA1HVM= +github.com/shoenig/test v0.4.0/go.mod h1:xYtyGBC5Q3kzCNyJg/SjgNpfAa2kvmgA0i5+lQso8x0= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= -github.com/stretchr/testify v1.7.1 h1:5TQK59W5E3v0r2duFAb7P95B6hEeOyEnHRa8MjYSMTY= -github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= @@ -22,9 +26,6 @@ golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= -gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v2 v2.2.8 h1:obN1ZagJSUGI0Ek/LBmuj4SNLPfIny3KsKFopxRdj10= gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= -gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo= -gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/tools/missing/main.go b/tools/missing/main.go index 60ba7cb50..5825b30dc 100644 --- a/tools/missing/main.go +++ b/tools/missing/main.go @@ -1,6 +1,7 @@ package main import ( + "encoding/json" "errors" "fmt" "io" @@ -10,7 +11,7 @@ import ( "sort" "strings" - "gopkg.in/yaml.v2" + "github.com/hashicorp/go-set" ) func main() { @@ -20,21 +21,28 @@ func main() { } } -type YamlFile struct { - Jobs struct { - TestPackages struct { - Strategy struct { - Matrix struct { - Packages []string `yaml:"pkg"` - } `yaml:"matrix"` - } `yaml:"strategy"` - } `yaml:"tests-pkgs"` - } `yaml:"jobs"` +// Manifest represents groupings of packages for testing +// see: ci/test-core.json +type Manifest map[string][]string + +func (m Manifest) covers(pkg string) bool { + for _, list := range m { + if isCovered(list, pkg) { + return true + } + } + return false } +const ( + verify = 1 + group = 2 +) + func run(args []string) error { - if len(args) != 1 { - return errors.New("requires filename") + mode := len(args) + if !(mode == verify || mode == group) { + return errors.New("usage: [filename] ") } f, err := os.Open(args[0]) @@ -45,11 +53,22 @@ func run(args []string) error { _ = f.Close() }() - coverage, err := inMatrix(f) + manifest, err := getManifest(f) if err != nil { return err } + switch mode { + case verify: + return runVerify(manifest) + case group: + return runGroups(manifest, args[1]) + default: + panic("oops") + } +} + +func runVerify(manifest Manifest) error { packages, err := inCode(".") if err != nil { return err @@ -57,7 +76,7 @@ func run(args []string) error { var isMissing []string for _, pkg := range packages { - if !isCovered(coverage, pkg) { + if !manifest.covers(pkg) { isMissing = append(isMissing, pkg) } } @@ -74,7 +93,17 @@ func run(args []string) error { return nil } -// isCovered returns true if pkg is covered by a package in coverage. +func runGroups(manifest Manifest, group string) error { + list := manifest[group] + for i := 0; i < len(list); i++ { + list[i] = "./" + list[i] + } + s := strings.Join(list, " ") + fmt.Print(s) + return nil +} + +// isCovered returns true if pkg is tested by directory in manifest. func isCovered(coverage []string, pkg string) bool { for _, p := range coverage { if isCoveredOne(p, pkg) { @@ -101,19 +130,14 @@ func isCoveredOne(p string, pkg string) bool { return false } -func inMatrix(r io.Reader) ([]string, error) { - var yFile YamlFile - if err := yaml.NewDecoder(r).Decode(&yFile); err != nil { +func getManifest(r io.Reader) (Manifest, error) { + m := make(Manifest) + if err := json.NewDecoder(r).Decode(&m); err != nil { return nil, err } - p := yFile.Jobs.TestPackages.Strategy.Matrix.Packages - return p, nil + return m, nil } -type nothing struct{} - -var null = nothing{} - // uninteresting lists remaining packages that contain Go code but still // do not need to be covered by test cases. var uninteresting = []string{ @@ -147,7 +171,7 @@ func skip(p string) bool { } func inCode(root string) ([]string, error) { - m := map[string]nothing{} + pkgs := set.New[string](100) err := filepath.Walk(root, func(path string, info fs.FileInfo, err error) error { if info.IsDir() { @@ -159,7 +183,7 @@ func inCode(root string) ([]string, error) { } if ext := filepath.Ext(path); ext == ".go" { - m[filepath.Dir(path)] = null + pkgs.Insert(filepath.Dir(path)) } return nil @@ -168,12 +192,9 @@ func inCode(root string) ([]string, error) { return nil, err } - delete(m, ".") // package main + pkgs.Remove(".") // main - var packages []string - for p := range m { - packages = append(packages, p) - } + packages := pkgs.List() sort.Strings(packages) return packages, nil } diff --git a/tools/missing/main_test.go b/tools/missing/main_test.go index 7b405b258..63494a410 100644 --- a/tools/missing/main_test.go +++ b/tools/missing/main_test.go @@ -3,13 +3,13 @@ package main import ( "testing" - "github.com/stretchr/testify/require" + "github.com/shoenig/test/must" ) func Test_isCoveredOne(t *testing.T) { try := func(p string, exp bool) { result := isCoveredOne(p, "foo/bar") - require.Equal(t, exp, result) + must.Eq(t, exp, result) } try("baz", false) try("foo", false)