Refactor reading turbo.json and add test cases (#1929)

Adds test cases for reading the legacy config from package.json

cli/internal/fs/testdata/both/package.json

@@ -0,0 +1,7 @@
+{
+  "turbo": {
+    "pipeline": {
+      "build": {}
+    }
+  }
+}

cli/internal/fs/testdata/both/turbo.json

@@ -0,0 +1,21 @@
+// mocked test comment
+{
+  "pipeline": {
+    "build": {
+      // mocked test comment
+      "dependsOn": [
+        // mocked test comment
+        "^build"
+      ],
+      "outputs": [
+        "dist/**",
+        ".next/**"
+      ],
+      "outputMode": "new-only"
+    } // mocked test comment
+  },
+  "remoteCache": {
+    "teamId": "team_id",
+    "signature": true
+  }
+}

cli/internal/fs/testdata/legacy-only/package.json

@@ -0,0 +1,7 @@
+{
+  "turbo": {
+    "pipeline": {
+      "build": {}
+    }
+  }
+}

cli/internal/fs/turbo_json.go

@@ -12,6 +12,14 @@ import (
 	"muzzammil.xyz/jsonc"
 )
 
+const (
+	configFile                   = "turbo.json"
+	envPipelineDelimiter         = "$"
+	topologicalPipelineDelimiter = "^"
+)
+
+var defaultOutputs = []string{"dist/**/*", "build/**/*"}
+
 // TurboJSON is the root turborepo configuration
 type TurboJSON struct {
 	// Global root filesystem dependencies
@@ -23,36 +31,68 @@ type TurboJSON struct {
 	RemoteCacheOptions RemoteCacheOptions `json:"remoteCache,omitempty"`
 }
 
-const configFile = "turbo.json"
+// RemoteCacheOptions is a struct for deserializing .remoteCache of configFile
+type RemoteCacheOptions struct {
+	TeamID    string `json:"teamId,omitempty"`
+	Signature bool   `json:"signature,omitempty"`
+}
+
+type pipelineJSON struct {
+	Outputs    *[]string           `json:"outputs"`
+	Cache      *bool               `json:"cache,omitempty"`
+	DependsOn  []string            `json:"dependsOn,omitempty"`
+	Inputs     []string            `json:"inputs,omitempty"`
+	OutputMode util.TaskOutputMode `json:"outputMode,omitempty"`
+}
+
+// Pipeline is a struct for deserializing .pipeline in configFile
+type Pipeline map[string]TaskDefinition
+
+// TaskDefinition is a representation of the configFile pipeline for further computation.
+type TaskDefinition struct {
+	Outputs                 []string
+	ShouldCache             bool
+	EnvVarDependencies      []string
+	TopologicalDependencies []string
+	TaskDependencies        []string
+	Inputs                  []string
+	OutputMode              util.TaskOutputMode
+}
 
 // ReadTurboConfig toggles between reading from package.json or the configFile to support early adopters.
 func ReadTurboConfig(rootPath turbopath.AbsolutePath, rootPackageJSON *PackageJSON) (*TurboJSON, error) {
-	// If the configFile exists, we use that
-	// If pkg.Turbo exists, we warn about running the migration
-	// Use pkg.Turbo if the configFile doesn't exist
-	// If neither exists, it's a fatal error
+
 	turboJSONPath := rootPath.Join(configFile)
 
-	if !turboJSONPath.FileExists() {
-		if rootPackageJSON.LegacyTurboConfig == nil {
-			// TODO: suggestion on how to create one
-			return nil, fmt.Errorf("Could not find %s. Follow directions at https://turborepo.org/docs/getting-started to create one", configFile)
+	// Check if turbo key in package.json exists
+	hasLegacyConfig := rootPackageJSON.LegacyTurboConfig != nil
+
+	// If the configFile exists, use that
+	if turboJSONPath.FileExists() {
+		turboJSON, err := readTurboJSON(turboJSONPath)
+		if err != nil {
+			return nil, fmt.Errorf("%s: %w", configFile, err)
 		}
-		log.Printf("[WARNING] Turbo configuration now lives in \"%s\". Migrate to %s by running \"npx @turbo/codemod create-turbo-config\"\n", configFile, configFile)
-		return rootPackageJSON.LegacyTurboConfig, nil
-	}
 
-	turboJSON, err := readTurboJSON(turboJSONPath)
-	if err != nil {
-		return nil, fmt.Errorf("%s: %w", configFile, err)
+		// If pkg.Turbo exists, log a warning and delete it from the representation
+		// TODO: turn off this warning eventually
+		if hasLegacyConfig {
+			log.Printf("[WARNING] Ignoring \"turbo\" key in package.json, using %s instead.", configFile)
+			rootPackageJSON.LegacyTurboConfig = nil
+		}
+
+		return turboJSON, nil
 	}
 
-	if rootPackageJSON.LegacyTurboConfig != nil {
-		log.Printf("[WARNING] Ignoring legacy \"turbo\" key in package.json, using %s instead. Consider deleting the \"turbo\" key from package.json\n", configFile)
-		rootPackageJSON.LegacyTurboConfig = nil
+	// Use pkg.Turbo if the configFile doesn't exist and we want the fallback feature
+	// TODO: turn this fallback off eventually
+	if hasLegacyConfig {
+		log.Printf("[DEPRECATED] \"turbo\" in package.json is deprecated. Migrate to %s by running \"npx @turbo/codemod create-turbo-config\"\n", configFile)
+		return rootPackageJSON.LegacyTurboConfig, nil
 	}
 
-	return turboJSON, nil
+	// If there's no turbo.json and no turbo key in package.json, return an error.
+	return nil, fmt.Errorf("Could not find %s. Follow directions at https://turborepo.org/docs/getting-started to create one", configFile)
 }
 
 // readTurboJSON reads the configFile in to a struct
@@ -74,23 +114,6 @@ func readTurboJSON(path turbopath.AbsolutePath) (*TurboJSON, error) {
 	return turboJSON, nil
 }
 
-// RemoteCacheOptions is a struct for deserializing .remoteCache of configFile
-type RemoteCacheOptions struct {
-	TeamID    string `json:"teamId,omitempty"`
-	Signature bool   `json:"signature,omitempty"`
-}
-
-type pipelineJSON struct {
-	Outputs    *[]string           `json:"outputs"`
-	Cache      *bool               `json:"cache,omitempty"`
-	DependsOn  []string            `json:"dependsOn,omitempty"`
-	Inputs     []string            `json:"inputs,omitempty"`
-	OutputMode util.TaskOutputMode `json:"outputMode,omitempty"`
-}
-
-// Pipeline is a struct for deserializing .pipeline in configFile
-type Pipeline map[string]TaskDefinition
-
 // GetTaskDefinition returns a TaskDefinition from a serialized definition in configFile
 func (pc Pipeline) GetTaskDefinition(taskID string) (TaskDefinition, bool) {
 	if entry, ok := pc[taskID]; ok {
@@ -118,24 +141,6 @@ func (pc Pipeline) HasTask(task string) bool {
 	return false
 }
 
-// TaskDefinition is a representation of the configFile pipeline for further computation.
-type TaskDefinition struct {
-	Outputs                 []string
-	ShouldCache             bool
-	EnvVarDependencies      []string
-	TopologicalDependencies []string
-	TaskDependencies        []string
-	Inputs                  []string
-	OutputMode              util.TaskOutputMode
-}
-
-const (
-	envPipelineDelimiter         = "$"
-	topologicalPipelineDelimiter = "^"
-)
-
-var defaultOutputs = []string{"dist/**/*", "build/**/*"}
-
 // UnmarshalJSON deserializes JSON into a TaskDefinition
 func (c *TaskDefinition) UnmarshalJSON(data []byte) error {
 	rawPipeline := &pipelineJSON{}

cli/internal/fs/turbo_json_test.go

@@ -6,29 +6,21 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/vercel/turborepo/cli/internal/turbopath"
 	"github.com/vercel/turborepo/cli/internal/util"
 )
 
 func Test_ReadTurboConfig(t *testing.T) {
-	defaultCwd, err := os.Getwd()
-	if err != nil {
-		t.Errorf("failed to get cwd: %v", err)
-	}
-	cwd, err := CheckedToAbsolutePath(defaultCwd)
-	if err != nil {
-		t.Fatalf("cwd is not an absolute directory %v: %v", defaultCwd, err)
-	}
+	testDir := getTestDir(t, "correct")
 
-	rootDir := "testdata"
-	turboJSONPath := cwd.Join(rootDir)
-	packageJSONPath := cwd.Join(rootDir, "package.json")
+	packageJSONPath := testDir.Join("package.json")
 	rootPackageJSON, pkgJSONReadErr := ReadPackageJSON(packageJSONPath)
 
 	if pkgJSONReadErr != nil {
 		t.Fatalf("invalid parse: %#v", pkgJSONReadErr)
 	}
 
-	turboJSON, turboJSONReadErr := ReadTurboConfig(turboJSONPath, rootPackageJSON)
+	turboJSON, turboJSONReadErr := ReadTurboConfig(testDir, rootPackageJSON)
 
 	if turboJSONReadErr != nil {
 		t.Fatalf("invalid parse: %#v", turboJSONReadErr)
@@ -70,24 +62,113 @@ func Test_ReadTurboConfig(t *testing.T) {
 		},
 	}
 
+	validateOutput(t, turboJSON.Pipeline, pipelineExpected)
+
 	remoteCacheOptionsExpected := RemoteCacheOptions{"team_id", true}
-	if len(turboJSON.Pipeline) != len(pipelineExpected) {
+	assert.EqualValues(t, remoteCacheOptionsExpected, turboJSON.RemoteCacheOptions)
+}
+
+func Test_ReadTurboConfig_Legacy(t *testing.T) {
+	testDir := getTestDir(t, "legacy-only")
+
+	packageJSONPath := testDir.Join("package.json")
+	rootPackageJSON, pkgJSONReadErr := ReadPackageJSON(packageJSONPath)
+
+	if pkgJSONReadErr != nil {
+		t.Fatalf("invalid parse: %#v", pkgJSONReadErr)
+	}
+
+	turboJSON, turboJSONReadErr := ReadTurboConfig(testDir, rootPackageJSON)
+
+	if turboJSONReadErr != nil {
+		t.Fatalf("invalid parse: %#v", turboJSONReadErr)
+	}
+
+	pipelineExpected := map[string]TaskDefinition{
+		"build": {
+			Outputs:                 []string{"dist/**/*", "build/**/*"},
+			TopologicalDependencies: []string{},
+			EnvVarDependencies:      []string{},
+			TaskDependencies:        []string{},
+			ShouldCache:             true,
+			OutputMode:              util.FullTaskOutput,
+		},
+	}
+
+	validateOutput(t, turboJSON.Pipeline, pipelineExpected)
+	assert.Empty(t, turboJSON.RemoteCacheOptions)
+}
+
+func Test_ReadTurboConfig_BothCorrectAndLegacy(t *testing.T) {
+	testDir := getTestDir(t, "both")
+
+	packageJSONPath := testDir.Join("package.json")
+	rootPackageJSON, pkgJSONReadErr := ReadPackageJSON(packageJSONPath)
+
+	if pkgJSONReadErr != nil {
+		t.Fatalf("invalid parse: %#v", pkgJSONReadErr)
+	}
+
+	turboJSON, turboJSONReadErr := ReadTurboConfig(testDir, rootPackageJSON)
+
+	if turboJSONReadErr != nil {
+		t.Fatalf("invalid parse: %#v", turboJSONReadErr)
+	}
+
+	pipelineExpected := map[string]TaskDefinition{
+		"build": {
+			Outputs:                 []string{"dist/**", ".next/**"},
+			TopologicalDependencies: []string{"build"},
+			EnvVarDependencies:      []string{},
+			TaskDependencies:        []string{},
+			ShouldCache:             true,
+			OutputMode:              util.NewTaskOutput,
+		},
+	}
+
+	validateOutput(t, turboJSON.Pipeline, pipelineExpected)
+
+	remoteCacheOptionsExpected := RemoteCacheOptions{"team_id", true}
+	assert.EqualValues(t, remoteCacheOptionsExpected, turboJSON.RemoteCacheOptions)
+
+	assert.Equal(t, rootPackageJSON.LegacyTurboConfig == nil, true)
+}
+
+// Helpers
+func validateOutput(t *testing.T, actual Pipeline, expected map[string]TaskDefinition) {
+	// check top level keys
+	if len(actual) != len(expected) {
 		expectedKeys := []string{}
-		for k := range pipelineExpected {
+		for k := range expected {
 			expectedKeys = append(expectedKeys, k)
 		}
 		actualKeys := []string{}
-		for k := range turboJSON.Pipeline {
+		for k := range actual {
 			actualKeys = append(actualKeys, k)
 		}
 		t.Errorf("pipeline tasks mismatch. got %v, want %v", strings.Join(actualKeys, ","), strings.Join(expectedKeys, ","))
 	}
-	for taskName, expectedTaskDefinition := range pipelineExpected {
-		actualTaskDefinition, ok := turboJSON.Pipeline[taskName]
+
+	// check individual task definitions
+	for taskName, expectedTaskDefinition := range expected {
+		actualTaskDefinition, ok := actual[taskName]
 		if !ok {
 			t.Errorf("missing expected task: %v", taskName)
 		}
 		assert.EqualValuesf(t, expectedTaskDefinition, actualTaskDefinition, "task definition mismatch for %v", taskName)
 	}
-	assert.EqualValues(t, remoteCacheOptionsExpected, turboJSON.RemoteCacheOptions)
+
+}
+
+func getTestDir(t *testing.T, testName string) turbopath.AbsolutePath {
+	defaultCwd, err := os.Getwd()
+	if err != nil {
+		t.Errorf("failed to get cwd: %v", err)
+	}
+	cwd, err := CheckedToAbsolutePath(defaultCwd)
+	if err != nil {
+		t.Fatalf("cwd is not an absolute directory %v: %v", defaultCwd, err)
+	}
+
+	return cwd.Join("testdata", testName)
 }