From 4e985048cbd5d97882afad3416c79c69548e1031 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Thu, 10 Apr 2025 13:55:29 -0700 Subject: [PATCH 1/2] Use an interator to walk FS for tests --- internal/testrunner/compiler_runner.go | 21 +++---- internal/testrunner/compiler_runner_test.go | 11 ++-- internal/testrunner/runner.go | 1 - internal/testutil/harnessutil/harnessutil.go | 58 +++++++++----------- 4 files changed, 41 insertions(+), 50 deletions(-) diff --git a/internal/testrunner/compiler_runner.go b/internal/testrunner/compiler_runner.go index 5f6c48091b..2739281cf4 100644 --- a/internal/testrunner/compiler_runner.go +++ b/internal/testrunner/compiler_runner.go @@ -70,21 +70,12 @@ func NewCompilerBaselineRunner(testType CompilerTestType, isSubmodule bool) *Com } } -func (r *CompilerBaselineRunner) EnumerateTestFiles() []string { - if len(r.testFiles) > 0 { - return r.testFiles - } - files, err := harnessutil.EnumerateFiles(r.basePath, compilerBaselineRegex, true /*recursive*/) - if err != nil { - panic("Could not read compiler test files: " + err.Error()) +func (r *CompilerBaselineRunner) RunTests(t *testing.T) { + if r.testFiles != nil { + panic("RunTests called multiple times") } - r.testFiles = files - return files -} -func (r *CompilerBaselineRunner) RunTests(t *testing.T) { r.cleanUpLocal(t) - files := r.EnumerateTestFiles() skippedTests := map[string]string{ "mappedTypeRecursiveInference.ts": "Skipped until we have type printer with truncation limit.", "jsFileCompilationWithoutJsExtensions.ts": "Skipped until we have proper allowJS support (and errors when not enabled.)", @@ -101,7 +92,11 @@ func (r *CompilerBaselineRunner) RunTests(t *testing.T) { "preserveValueImports_importsNotUsedAsValues.ts", "importsNotUsedAsValues_error.ts", } - for _, filename := range files { + for filename, err := range harnessutil.EnumerateFiles(r.basePath, compilerBaselineRegex, true /*recursive*/) { + assert.NilError(t, err, "Could not enumerate test files") + + r.testFiles = append(r.testFiles, filename) + if msg, ok := skippedTests[tspath.GetBaseFileName(filename)]; ok { t.Run(tspath.GetBaseFileName(filename), func(t *testing.T) { t.Skip(msg) }) continue diff --git a/internal/testrunner/compiler_runner_test.go b/internal/testrunner/compiler_runner_test.go index 910e00b942..80ed926dbf 100644 --- a/internal/testrunner/compiler_runner_test.go +++ b/internal/testrunner/compiler_runner_test.go @@ -35,16 +35,17 @@ func runCompilerTests(t *testing.T, isSubmodule bool) { NewCompilerBaselineRunner(TestTypeConformance, isSubmodule), } + for _, runner := range runners { + runner.RunTests(t) + } + + var seenTests core.Set[string] for _, runner := range runners { - for _, test := range runner.EnumerateTestFiles() { + for _, test := range runner.testFiles { test = tspath.GetBaseFileName(test) assert.Assert(t, !seenTests.Has(test), "Duplicate test file: %s", test) seenTests.Add(test) } } - - for _, runner := range runners { - runner.RunTests(t) - } } diff --git a/internal/testrunner/runner.go b/internal/testrunner/runner.go index f50d4f98c2..29f1d4c2c0 100644 --- a/internal/testrunner/runner.go +++ b/internal/testrunner/runner.go @@ -3,7 +3,6 @@ package runner import "testing" type Runner interface { - EnumerateTestFiles() []string RunTests(t *testing.T) } diff --git a/internal/testutil/harnessutil/harnessutil.go b/internal/testutil/harnessutil/harnessutil.go index f3ac5522d3..516557c86c 100644 --- a/internal/testutil/harnessutil/harnessutil.go +++ b/internal/testutil/harnessutil/harnessutil.go @@ -3,6 +3,7 @@ package harnessutil import ( "fmt" "io/fs" + "iter" "maps" "os" "path/filepath" @@ -789,40 +790,35 @@ func createProgram(host compiler.CompilerHost, options *core.CompilerOptions, ro return program } -func EnumerateFiles(folder string, testRegex *regexp.Regexp, recursive bool) ([]string, error) { - files, err := listFiles(folder, testRegex, recursive) - if err != nil { - return nil, err - } - return core.Map(files, tspath.NormalizeSlashes), nil -} - -func listFiles(path string, spec *regexp.Regexp, recursive bool) ([]string, error) { - return listFilesWorker(spec, recursive, path) -} - -func listFilesWorker(spec *regexp.Regexp, recursive bool, folder string) ([]string, error) { - folder = tspath.GetNormalizedAbsolutePath(folder, repo.TestDataPath) - entries, err := os.ReadDir(folder) - if err != nil { - return nil, err - } - var paths []string - for _, entry := range entries { - path := tspath.NormalizePath(filepath.Join(folder, entry.Name())) - if !entry.IsDir() { - if spec == nil || spec.MatchString(path) { - paths = append(paths, path) - } - } else if recursive { - subPaths, err := listFilesWorker(spec, recursive, path) - if err != nil { - return nil, err +func EnumerateFiles(folder string, testRegex *regexp.Regexp, recursive bool) iter.Seq2[string, error] { + return func(yield func(string, error) bool) { + folder = tspath.GetNormalizedAbsolutePath(folder, repo.TestDataPath) + entries, err := os.ReadDir(folder) + if err != nil { + yield("", err) + return + } + for _, entry := range entries { + path := tspath.NormalizePath(filepath.Join(folder, entry.Name())) + if !entry.IsDir() { + if testRegex == nil || testRegex.MatchString(path) { + if !yield(path, nil) { + return + } + } + } else if recursive { + for subPath, err := range EnumerateFiles(path, testRegex, recursive) { + if err != nil { + yield("", err) + return + } + if !yield(subPath, nil) { + return + } + } } - paths = append(paths, subPaths...) } } - return paths, nil } func getFileBasedTestConfigurationDescription(config TestConfiguration) string { From 15e74bf036abc41ca878bb96a5d0586e07ad67ce Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Thu, 10 Apr 2025 14:11:55 -0700 Subject: [PATCH 2/2] getCompilerFileBasedTest in parallel --- internal/testrunner/compiler_runner.go | 43 +++++++++-- internal/testutil/harnessutil/harnessutil.go | 76 ++++++++++++-------- 2 files changed, 83 insertions(+), 36 deletions(-) diff --git a/internal/testrunner/compiler_runner.go b/internal/testrunner/compiler_runner.go index 2739281cf4..5136f0883d 100644 --- a/internal/testrunner/compiler_runner.go +++ b/internal/testrunner/compiler_runner.go @@ -8,6 +8,7 @@ import ( "regexp" "slices" "strings" + "sync" "testing" "github.com/microsoft/typescript-go/internal/checker" @@ -92,6 +93,16 @@ func (r *CompilerBaselineRunner) RunTests(t *testing.T) { "preserveValueImports_importsNotUsedAsValues.ts", "importsNotUsedAsValues_error.ts", } + + type compilerFileBasedTestOrError struct { + filename string + test *compilerFileBasedTest + err error + } + + tests := make(chan compilerFileBasedTestOrError) + var wg sync.WaitGroup + for filename, err := range harnessutil.EnumerateFiles(r.basePath, compilerBaselineRegex, true /*recursive*/) { assert.NilError(t, err, "Could not enumerate test files") @@ -104,7 +115,25 @@ func (r *CompilerBaselineRunner) RunTests(t *testing.T) { if slices.Contains(deprecatedTests, tspath.GetBaseFileName(filename)) { continue } - r.runTest(t, filename) + + wg.Add(1) + go func() { + defer wg.Done() + test, err := getCompilerFileBasedTest(filename) + tests <- compilerFileBasedTestOrError{filename, test, err} + }() + } + + go func() { + wg.Wait() + close(tests) + }() + + for test := range tests { + if test.err != nil { + t.Fatalf("Failed to get test for %s: %v", test.filename, test.err) + } + r.runTest(t, test.filename, test.test) } } @@ -148,8 +177,7 @@ func getCompilerVaryByMap() map[string]struct{} { return varyByMap } -func (r *CompilerBaselineRunner) runTest(t *testing.T, filename string) { - test := getCompilerFileBasedTest(t, filename) +func (r *CompilerBaselineRunner) runTest(t *testing.T, filename string, test *compilerFileBasedTest) { basename := tspath.GetBaseFileName(filename) if len(test.configurations) > 0 { for _, config := range test.configurations { @@ -187,18 +215,21 @@ type compilerFileBasedTest struct { configurations []*harnessutil.NamedTestConfiguration } -func getCompilerFileBasedTest(t *testing.T, filename string) *compilerFileBasedTest { +func getCompilerFileBasedTest(filename string) (*compilerFileBasedTest, error) { content, ok := osvfs.FS().ReadFile(filename) if !ok { panic("Could not read test file: " + filename) } settings := extractCompilerSettings(content) - configurations := harnessutil.GetFileBasedTestConfigurations(t, settings, compilerVaryBy) + configurations, err := harnessutil.GetFileBasedTestConfigurations(settings, compilerVaryBy) + if err != nil { + return nil, fmt.Errorf("failed to get configurations for %s: %w", filename, err) + } return &compilerFileBasedTest{ filename: filename, content: content, configurations: configurations, - } + }, nil } type compilerTest struct { diff --git a/internal/testutil/harnessutil/harnessutil.go b/internal/testutil/harnessutil/harnessutil.go index 516557c86c..aa49075690 100644 --- a/internal/testutil/harnessutil/harnessutil.go +++ b/internal/testutil/harnessutil/harnessutil.go @@ -1,6 +1,7 @@ package harnessutil import ( + "errors" "fmt" "io/fs" "iter" @@ -29,6 +30,7 @@ import ( "github.com/microsoft/typescript-go/internal/tspath" "github.com/microsoft/typescript-go/internal/vfs" "github.com/microsoft/typescript-go/internal/vfs/vfstest" + "gotest.tools/v3/assert" ) // Posix-style path to additional test libraries @@ -246,7 +248,8 @@ func setOptionsFromTestConfig(t *testing.T, testConfig TestConfiguration, compil commandLineOption := getCommandLineOption(name) if commandLineOption != nil { - parsedValue := getOptionValue(t, commandLineOption, value) + parsedValue, err := getOptionValue(commandLineOption, value) + assert.NilError(t, err) errors := tsoptions.ParseCompilerOptions(commandLineOption.Name, parsedValue, compilerOptions) if len(errors) > 0 { t.Fatalf("Error parsing value '%s' for compiler option '%s'.", value, commandLineOption.Name) @@ -255,7 +258,8 @@ func setOptionsFromTestConfig(t *testing.T, testConfig TestConfiguration, compil } harnessOption := getHarnessOption(name) if harnessOption != nil { - parsedValue := getOptionValue(t, harnessOption, value) + parsedValue, err := getOptionValue(harnessOption, value) + assert.NilError(t, err) parseHarnessOption(t, harnessOption.Name, parsedValue, harnessOptions) continue } @@ -389,41 +393,41 @@ func parseHarnessOption(t *testing.T, key string, value any, options *HarnessOpt var deprecatedModuleResolution []string = []string{"node", "classic", "node10"} -func getOptionValue(t *testing.T, option *tsoptions.CommandLineOption, value string) tsoptions.CompilerOptionsValue { +func getOptionValue(option *tsoptions.CommandLineOption, value string) (tsoptions.CompilerOptionsValue, error) { switch option.Kind { case tsoptions.CommandLineOptionTypeString: - return value + return value, nil case tsoptions.CommandLineOptionTypeNumber: numVal, err := strconv.Atoi(value) if err != nil { - t.Fatalf("Value for option '%s' must be a number, got: %v", option.Name, value) + return nil, fmt.Errorf("Value for option '%s' must be a number, got: %v", option.Name, value) } - return numVal + return numVal, nil case tsoptions.CommandLineOptionTypeBoolean: switch strings.ToLower(value) { case "true": - return true + return true, nil case "false": - return false + return false, nil default: - t.Fatalf("Value for option '%s' must be a boolean, got: %v", option.Name, value) + return nil, fmt.Errorf("Value for option '%s' must be a boolean, got: %v", option.Name, value) } case tsoptions.CommandLineOptionTypeEnum: enumVal, ok := option.EnumMap().Get(strings.ToLower(value)) if !ok { - t.Fatalf("Value for option '%s' must be one of %s, got: %v", option.Name, strings.Join(slices.Collect(option.EnumMap().Keys()), ","), value) + return nil, fmt.Errorf("Value for option '%s' must be one of %s, got: %v", option.Name, strings.Join(slices.Collect(option.EnumMap().Keys()), ","), value) } - return enumVal + return enumVal, nil case tsoptions.CommandLineOptionTypeList, tsoptions.CommandLineOptionTypeListOrElement: listVal, errors := tsoptions.ParseListTypeOption(option, value) if len(errors) > 0 { - t.Fatalf("Unknown value '%s' for compiler option '%s'", value, option.Name) + return nil, fmt.Errorf("Error parsing value '%s' for compiler option '%s': %v", value, option.Name, errors) } - return listVal + return listVal, nil case tsoptions.CommandLineOptionTypeObject: - t.Fatalf("Object type options like '%s' are not supported", option.Name) + return nil, fmt.Errorf("Object type options like '%s' are not supported", option.Name) } - return nil + return nil, nil } type cachedCompilerHost struct { @@ -833,17 +837,20 @@ func getFileBasedTestConfigurationDescription(config TestConfiguration) string { return output.String() } -func GetFileBasedTestConfigurations(t *testing.T, settings map[string]string, varyByOptions map[string]struct{}) []*NamedTestConfiguration { +func GetFileBasedTestConfigurations(settings map[string]string, varyByOptions map[string]struct{}) ([]*NamedTestConfiguration, error) { var optionEntries [][]string // Each element slice has the option name as the first element, and the values as the rest variationCount := 1 nonVaryingOptions := make(map[string]string) for option, value := range settings { if _, ok := varyByOptions[option]; ok { - entries := splitOptionValues(t, value, option) + entries, err := splitOptionValues(value, option) + if err != nil { + return nil, err + } if len(entries) > 1 { variationCount *= len(entries) if variationCount > 25 { - t.Fatal("Provided test options exceeded the maximum number of variations") + return nil, errors.New("Provided test options exceeded the maximum number of variations") } optionEntries = append(optionEntries, append([]string{option}, entries...)) } else if len(entries) == 1 { @@ -868,7 +875,7 @@ func GetFileBasedTestConfigurations(t *testing.T, settings map[string]string, va // Only non-varying options configurations = append(configurations, &NamedTestConfiguration{"", nonVaryingOptions}) } - return configurations + return configurations, nil } // Splits a string value into an array of strings, each corresponding to a unique value for the given option. @@ -880,9 +887,9 @@ func GetFileBasedTestConfigurations(t *testing.T, settings map[string]string, va // splitOptionValues("*, -true", "strict") => ["false"] // // ``` -func splitOptionValues(t *testing.T, value string, option string) []string { +func splitOptionValues(value string, option string) ([]string, error) { if len(value) == 0 { - return nil + return nil, nil } star := false @@ -903,7 +910,7 @@ func splitOptionValues(t *testing.T, value string, option string) []string { } if len(includes) == 0 && !star && len(excludes) == 0 { - return nil + return nil, nil } // Dedupe the variations by their normalized values @@ -911,7 +918,10 @@ func splitOptionValues(t *testing.T, value string, option string) []string { // add (and deduplicate) all included entries for _, include := range includes { - value := getValueOfOptionString(t, option, include) + value, err := getValueOfOptionString(option, include) + if err != nil { + return nil, err + } if _, ok := variations[value]; !ok { variations[value] = include } @@ -921,7 +931,10 @@ func splitOptionValues(t *testing.T, value string, option string) []string { if star && len(allValues) > 0 { // add all entries for _, include := range allValues { - value := getValueOfOptionString(t, option, include) + value, err := getValueOfOptionString(option, include) + if err != nil { + return nil, err + } if _, ok := variations[value]; !ok { variations[value] = include } @@ -930,26 +943,29 @@ func splitOptionValues(t *testing.T, value string, option string) []string { // remove all excluded entries for _, exclude := range excludes { - value := getValueOfOptionString(t, option, exclude) + value, err := getValueOfOptionString(option, exclude) + if err != nil { + return nil, err + } delete(variations, value) } if len(variations) == 0 { panic(fmt.Sprintf("Variations in test option '@%s' resulted in an empty set.", option)) } - return slices.Collect(maps.Values(variations)) + return slices.AppendSeq(make([]string, 0, len(variations)), maps.Values(variations)), nil } -func getValueOfOptionString(t *testing.T, option string, value string) tsoptions.CompilerOptionsValue { +func getValueOfOptionString(option string, value string) (tsoptions.CompilerOptionsValue, error) { optionDecl := getCommandLineOption(option) if optionDecl == nil { - t.Fatalf("Unknown option '%s'", option) + return nil, fmt.Errorf("Unknown option '%s'", option) } // TODO(gabritto): remove this when we deprecate the tests containing those option values if optionDecl.Name == "moduleResolution" && slices.Contains(deprecatedModuleResolution, strings.ToLower(value)) { - return value + return value, nil } - return getOptionValue(t, optionDecl, value) + return getOptionValue(optionDecl, value) } func getCommandLineOption(option string) *tsoptions.CommandLineOption {