From 5482e16a82170d2dc1677e0fce2489c860f3ab36 Mon Sep 17 00:00:00 2001 From: Alec Thomas Date: Sun, 18 Aug 2024 08:44:11 +1000 Subject: [PATCH] fix(integration): create a new test dir per language (#2413) Fixes #2412 --- .github/workflows/ci.yml | 8 +-- Justfile | 2 +- internal/buildengine/build_go_test.go | 6 +- internal/buildengine/build_test.go | 2 + .../buildengine/testdata/depcycle1/go.mod | 2 +- .../buildengine/testdata/depcycle2/go.mod | 2 +- internal/integration/harness.go | 57 ++++++++++--------- jvm-runtime/jvm_integration_test.go | 1 - 8 files changed, 43 insertions(+), 37 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c085e9bb51..0e5cfbb837 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,7 +20,7 @@ jobs: run: docker compose up -d --wait - name: Test run: | - go-test-annotate ${{ (github.event_name == 'pull_request' && github.event.action != 'enqueued') && '-short' || '' }} + go-test-annotate ${{ (github.event_name == 'pull_request' && github.event.action != 'enqueued' && !contains( github.event.pull_request.labels.*.name, 'run-all')) && '-short' || '' }} test-readme: name: Test README runs-on: ubuntu-latest @@ -153,7 +153,7 @@ jobs: run: just build-extension build-all: name: Rebuild All - if: github.event_name != 'pull_request' || github.event.action == 'enqueued' + if: github.event_name != 'pull_request' || github.event.action == 'enqueued' || contains( github.event.pull_request.labels.*.name, 'run-all') runs-on: ubuntu-latest steps: - name: Checkout code @@ -188,7 +188,7 @@ jobs: - run: cd docs && zola build integration-shard: name: Shard Integration Tests - if: github.event_name != 'pull_request' || github.event.action == 'enqueued' || contains( github.event.pull_request.labels.*.name, 'run-integration') + if: github.event_name != 'pull_request' || github.event.action == 'enqueued' || contains( github.event.pull_request.labels.*.name, 'run-all') runs-on: ubuntu-latest outputs: matrix: ${{ steps.extract-tests.outputs.matrix }} @@ -204,7 +204,7 @@ jobs: echo "matrix={\"test\":$(jq -c -n '$ARGS.positional' --args $(git grep -l '^//go:build integration' | xargs grep '^func Test' | awk '{print $2}' | cut -d'(' -f1))}" >> "$GITHUB_OUTPUT" integration-run: name: Integration Test - if: github.event_name != 'pull_request' || github.event.action == 'enqueued' || contains( github.event.pull_request.labels.*.name, 'run-integration') + if: github.event_name != 'pull_request' || github.event.action == 'enqueued' || contains( github.event.pull_request.labels.*.name, 'run-all') needs: integration-shard runs-on: ubuntu-latest strategy: diff --git a/Justfile b/Justfile index 355402b84e..f0c43d8ea5 100644 --- a/Justfile +++ b/Justfile @@ -79,7 +79,7 @@ build-sqlc: # Build the ZIP files that are embedded in the FTL release binaries build-zips: build-kt-runtime - @for dir in {{ZIP_DIRS}}; do (cd $dir && mk ../$(basename ${dir}).zip : . -- "rm -f $(basename ${dir}.zip) && zip -q --symlinks -r ../$(basename ${dir}).zip ."); done + @for dir in {{ZIP_DIRS}}; do (echo "cd $dir" && cd $dir && mk ../$(basename ${dir}).zip : . -- "rm -f $(basename ${dir}.zip) && zip -q --symlinks -r ../$(basename ${dir}).zip ."); done # Rebuild frontend build-frontend: npm-install diff --git a/internal/buildengine/build_go_test.go b/internal/buildengine/build_go_test.go index 8f324379c6..f31b89fc6f 100644 --- a/internal/buildengine/build_go_test.go +++ b/internal/buildengine/build_go_test.go @@ -74,9 +74,9 @@ func TestGoModVersion(t *testing.T) { } func TestGeneratedTypeRegistry(t *testing.T) { - if testing.Short() { - t.SkipNow() - } + // if testing.Short() { + // t.SkipNow() + // } sch := &schema.Schema{ Modules: []*schema.Module{ {Name: "another", Decls: []schema.Decl{ diff --git a/internal/buildengine/build_test.go b/internal/buildengine/build_test.go index e28a4ee598..035b3e4173 100644 --- a/internal/buildengine/build_test.go +++ b/internal/buildengine/build_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/alecthomas/assert/v2" + "github.com/alecthomas/repr" "github.com/TBD54566975/ftl/backend/schema" "github.com/TBD54566975/ftl/internal/log" @@ -57,6 +58,7 @@ func testBuild( assert.NoError(t, err, "Error loading project config") configs = append(configs, config) } + repr.Println(configs) // generate stubs to create the shared modules directory err = GenerateStubs(ctx, projectRootDir, bctx.sch.Modules, configs) diff --git a/internal/buildengine/testdata/depcycle1/go.mod b/internal/buildengine/testdata/depcycle1/go.mod index 248f9f3b94..4575c553d9 100644 --- a/internal/buildengine/testdata/depcycle1/go.mod +++ b/internal/buildengine/testdata/depcycle1/go.mod @@ -2,4 +2,4 @@ module ftl/depcycle1 go 1.22.2 -replace github.com/TBD54566975/ftl => ./../../../../.. +replace github.com/TBD54566975/ftl => ../../../.. diff --git a/internal/buildengine/testdata/depcycle2/go.mod b/internal/buildengine/testdata/depcycle2/go.mod index abba7b70cc..80f766d8b9 100644 --- a/internal/buildengine/testdata/depcycle2/go.mod +++ b/internal/buildengine/testdata/depcycle2/go.mod @@ -2,4 +2,4 @@ module ftl/depcycle2 go 1.22.2 -replace github.com/TBD54566975/ftl => ./../../../.. +replace github.com/TBD54566975/ftl => ./../../.. diff --git a/internal/integration/harness.go b/internal/integration/harness.go index 140e9dd7b9..0dd23b7793 100644 --- a/internal/integration/harness.go +++ b/internal/integration/harness.go @@ -138,38 +138,12 @@ func run(t *testing.T, actionsOrOptions ...ActionOrOption) { t.Setenv(key, value) } - tmpDir := t.TempDir() - cwd, err := os.Getwd() assert.NoError(t, err) rootDir, ok := internal.GitRoot("").Get() assert.True(t, ok) - if opts.ftlConfigPath != "" { - // TODO: We shouldn't be copying the shared config from the "go" testdata... - opts.ftlConfigPath = filepath.Join(cwd, "testdata", "go", opts.ftlConfigPath) - projectPath := filepath.Join(tmpDir, "ftl-project.toml") - - // Copy the specified FTL config to the temporary directory. - err = copy.Copy(opts.ftlConfigPath, projectPath) - if err == nil { - t.Setenv("FTL_CONFIG", projectPath) - } else { - // Use a path into the testdata directory instead of one relative to - // tmpDir. Otherwise we have a chicken and egg situation where the config - // can't be loaded until the module is copied over, and the config itself - // is used by FTL during startup. - // Some tests still rely on this behavior, so we can't remove it entirely. - t.Logf("Failed to copy %s to %s: %s", opts.ftlConfigPath, projectPath, err) - t.Setenv("FTL_CONFIG", opts.ftlConfigPath) - } - - } else { - err = os.WriteFile(filepath.Join(tmpDir, "ftl-project.toml"), []byte(`name = "integration"`), 0644) - assert.NoError(t, err) - } - // Build FTL binary logger := log.Configure(&logWriter{logger: t}, log.Config{Level: log.Debug}) ctx := log.ContextWithLogger(context.Background(), logger) @@ -188,6 +162,8 @@ func run(t *testing.T, actionsOrOptions ...ActionOrOption) { for _, language := range opts.languages { ctx, done := context.WithCancel(ctx) t.Run(language, func(t *testing.T) { + tmpDir := initWorkDir(t, cwd, opts) + verbs := rpc.Dial(ftlv1connect.NewVerbServiceClient, "http://localhost:8892", log.Debug) var controller ftlv1connect.ControllerServiceClient @@ -232,6 +208,35 @@ func run(t *testing.T, actionsOrOptions ...ActionOrOption) { } } +func initWorkDir(t testing.TB, cwd string, opts options) string { + tmpDir := t.TempDir() + + if opts.ftlConfigPath != "" { + // TODO: We shouldn't be copying the shared config from the "go" testdata... + opts.ftlConfigPath = filepath.Join(cwd, "testdata", "go", opts.ftlConfigPath) + projectPath := filepath.Join(tmpDir, "ftl-project.toml") + + // Copy the specified FTL config to the temporary directory. + err := copy.Copy(opts.ftlConfigPath, projectPath) + if err == nil { + t.Setenv("FTL_CONFIG", projectPath) + } else { + // Use a path into the testdata directory instead of one relative to + // tmpDir. Otherwise we have a chicken and egg situation where the config + // can't be loaded until the module is copied over, and the config itself + // is used by FTL during startup. + // Some tests still rely on this behavior, so we can't remove it entirely. + t.Logf("Failed to copy %s to %s: %s", opts.ftlConfigPath, projectPath, err) + t.Setenv("FTL_CONFIG", opts.ftlConfigPath) + } + + } else { + err := os.WriteFile(filepath.Join(tmpDir, "ftl-project.toml"), []byte(`name = "integration"`), 0644) + assert.NoError(t, err) + } + return tmpDir +} + type TestContext struct { context.Context // Temporary directory the test is executing in. diff --git a/jvm-runtime/jvm_integration_test.go b/jvm-runtime/jvm_integration_test.go index 31f2e48e43..4038b8b24e 100644 --- a/jvm-runtime/jvm_integration_test.go +++ b/jvm-runtime/jvm_integration_test.go @@ -19,7 +19,6 @@ func TestLifecycle(t *testing.T) { in.WithLanguages("java", "kotlin"), in.GitInit(), in.Exec("rm", "ftl-project.toml"), - in.IfLanguage("kotlin", in.Exec("rm", "-r", "echo")), //horrible, but we need to do cleanup, I wonder if we should be running each test in a separate directory in.Exec("ftl", "init", "test", "."), in.IfLanguage("java", in.Exec("ftl", "new", "java", ".", "echo")), in.IfLanguage("kotlin", in.Exec("ftl", "new", "kotlin", ".", "echo")),