From 5b6bbb1c31facf954b0ca02d79e0a5ca900c9e7a Mon Sep 17 00:00:00 2001 From: Eik Slowhand Madsen Date: Fri, 13 Mar 2020 21:02:09 +0100 Subject: [PATCH 1/7] - increase timeout on init --- harness/app.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/harness/app.go b/harness/app.go index f36a42a2..e287c1a5 100644 --- a/harness/app.go +++ b/harness/app.go @@ -73,7 +73,7 @@ func (cmd AppCmd) Start() error { case <-cmd.waitChan(): return errors.New("revel/harness: app died") - case <-time.After(30 * time.Second): + case <-time.After(90 * time.Second): cmd.Kill() return errors.New("revel/harness: app timed out") From 951f6b41899b4b0fb47fd82c95aeb08ced113441 Mon Sep 17 00:00:00 2001 From: Eik Slowhand Madsen Date: Fri, 13 Mar 2020 21:04:25 +0100 Subject: [PATCH 2/7] - parallelize test suites --- revel/test.go | 103 ++++++++++++++++++++++++++++---------------------- 1 file changed, 57 insertions(+), 46 deletions(-) diff --git a/revel/test.go b/revel/test.go index e7135a2d..c75d619f 100644 --- a/revel/test.go +++ b/revel/test.go @@ -13,6 +13,7 @@ import ( "os" "path/filepath" "strings" + "sync" "time" "github.com/revel/cmd/harness" @@ -226,8 +227,11 @@ func getTestsList(baseURL string) (*[]controllers.TestSuiteDesc, error) { break } } - if i < 3 { - time.Sleep(3 * time.Second) + if resp.Body != nil { + resp.Body.Close() + } + if i < 20 { + time.Sleep(time.Millisecond * 500) continue } if err != nil { @@ -261,54 +265,61 @@ func runTestSuites(baseURL, resultPath string, testSuites *[]controllers.TestSui overallSuccess = true failedResults []controllers.TestSuiteResult ) - for _, suite := range *testSuites { - // Print the name of the suite we're running. - name := suite.Name - if len(name) > 22 { - name = name[:19] + "..." - } - fmt.Printf("%-22s", name) + var wg sync.WaitGroup + // TODO make this configurable and default to 1 + parallelism := make(chan struct{}, 50) + for _, s := range *testSuites { + wg.Add(1) + parallelism <- struct{}{} + go func(suite controllers.TestSuiteDesc) { + defer wg.Done() + defer func() { <-parallelism }() + // Print the name of the suite we're running. + name := suite.Name + if len(name) > 22 { + name = name[:19] + "..." + } + fmt.Printf("%-22s", name) + + // Run every test. + startTime := time.Now() + suiteResult := controllers.TestSuiteResult{Name: suite.Name, Passed: true} + for _, test := range suite.Tests { + testURL := baseURL + "/@tests/" + suite.Name + "/" + test.Name + resp, err := http.Get(testURL) + if err != nil { + errorf("Failed to fetch test result at url %s: %s", testURL, err) + } - // Run every test. - startTime := time.Now() - suiteResult := controllers.TestSuiteResult{Name: suite.Name, Passed: true} - for _, test := range suite.Tests { - testURL := baseURL + "/@tests/" + suite.Name + "/" + test.Name - resp, err := http.Get(testURL) + var testResult controllers.TestResult + err = json.NewDecoder(resp.Body).Decode(&testResult) + resp.Body.Close() + if err == nil && !testResult.Passed { + suiteResult.Passed = false + } + suiteResult.Results = append(suiteResult.Results, testResult) + } + overallSuccess = overallSuccess && suiteResult.Passed + + // Print result. (Just PASSED or FAILED, and the time taken) + suiteResultStr, suiteAlert := "PASSED", "" + if !suiteResult.Passed { + suiteResultStr, suiteAlert = "FAILED", "!" + failedResults = append(failedResults, suiteResult) + } + fmt.Printf("%8s%3s%6ds\n", suiteResultStr, suiteAlert, int(time.Since(startTime).Seconds())) + // Create the result HTML file. + suiteResultFilename := filepath.Join(resultPath, + fmt.Sprintf("%s.%s.html", suite.Name, strings.ToLower(suiteResultStr))) + suiteResultFile, err := os.Create(suiteResultFilename) if err != nil { - errorf("Failed to fetch test result at url %s: %s", testURL, err) + errorf("Failed to create result file %s: %s", suiteResultFilename, err) } - defer func() { - _ = resp.Body.Close() - }() - - var testResult controllers.TestResult - err = json.NewDecoder(resp.Body).Decode(&testResult) - if err == nil && !testResult.Passed { - suiteResult.Passed = false + if err = resultTemplate.Render(suiteResultFile, suiteResult); err != nil { + errorf("Failed to render result template: %s", err) } - suiteResult.Results = append(suiteResult.Results, testResult) - } - overallSuccess = overallSuccess && suiteResult.Passed - - // Print result. (Just PASSED or FAILED, and the time taken) - suiteResultStr, suiteAlert := "PASSED", "" - if !suiteResult.Passed { - suiteResultStr, suiteAlert = "FAILED", "!" - failedResults = append(failedResults, suiteResult) - } - fmt.Printf("%8s%3s%6ds\n", suiteResultStr, suiteAlert, int(time.Since(startTime).Seconds())) - // Create the result HTML file. - suiteResultFilename := filepath.Join(resultPath, - fmt.Sprintf("%s.%s.html", suite.Name, strings.ToLower(suiteResultStr))) - suiteResultFile, err := os.Create(suiteResultFilename) - if err != nil { - errorf("Failed to create result file %s: %s", suiteResultFilename, err) - } - if err = resultTemplate.Render(suiteResultFile, suiteResult); err != nil { - errorf("Failed to render result template: %s", err) - } + }(s) } - + wg.Wait() return &failedResults, overallSuccess } From 8733f51d17a2d79246fce85f7c295d7a2f6641ba Mon Sep 17 00:00:00 2001 From: Eik Slowhand Madsen Date: Sun, 15 Mar 2020 21:04:25 +0100 Subject: [PATCH 3/7] - async proof a bit --- revel/test.go | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/revel/test.go b/revel/test.go index c75d619f..1358fd0b 100644 --- a/revel/test.go +++ b/revel/test.go @@ -261,10 +261,8 @@ func runTestSuites(baseURL, resultPath string, testSuites *[]controllers.TestSui errorf("Failed to load suite result template: %s", err) } - var ( - overallSuccess = true - failedResults []controllers.TestSuiteResult - ) + failedResults := make(chan controllers.TestSuiteResult, len(*testSuites)) + var wg sync.WaitGroup // TODO make this configurable and default to 1 parallelism := make(chan struct{}, 50) @@ -299,18 +297,15 @@ func runTestSuites(baseURL, resultPath string, testSuites *[]controllers.TestSui } suiteResult.Results = append(suiteResult.Results, testResult) } - overallSuccess = overallSuccess && suiteResult.Passed - // Print result. (Just PASSED or FAILED, and the time taken) suiteResultStr, suiteAlert := "PASSED", "" if !suiteResult.Passed { suiteResultStr, suiteAlert = "FAILED", "!" - failedResults = append(failedResults, suiteResult) + failedResults <- suiteResult } - fmt.Printf("%8s%3s%6ds\n", suiteResultStr, suiteAlert, int(time.Since(startTime).Seconds())) + // Create the result HTML file. - suiteResultFilename := filepath.Join(resultPath, - fmt.Sprintf("%s.%s.html", suite.Name, strings.ToLower(suiteResultStr))) + suiteResultFilename := filepath.Join(resultPath, fmt.Sprintf("%s.%s.html", suite.Name, strings.ToLower(suiteResultStr))) suiteResultFile, err := os.Create(suiteResultFilename) if err != nil { errorf("Failed to create result file %s: %s", suiteResultFilename, err) @@ -318,8 +313,16 @@ func runTestSuites(baseURL, resultPath string, testSuites *[]controllers.TestSui if err = resultTemplate.Render(suiteResultFile, suiteResult); err != nil { errorf("Failed to render result template: %s", err) } + + // Print result. (Just PASSED or FAILED, and the time taken) + fmt.Printf("%8s%3s%6ds\n", suiteResultStr, suiteAlert, int(time.Since(startTime).Seconds())) }(s) } wg.Wait() - return &failedResults, overallSuccess + close(failedResults) + failedResultsSlice := make([]controllers.TestSuiteResult, len(failedResults)) + for fr := range failedResults { + failedResultsSlice = append(failedResultsSlice, fr) + } + return &failedResultsSlice, len(failedResultsSlice) == 0 } From deb337ef96962fbe24f0e388c58349d076b1ce9a Mon Sep 17 00:00:00 2001 From: Eik Slowhand Madsen Date: Sun, 5 Apr 2020 16:34:27 +0200 Subject: [PATCH 4/7] - copy in original to lower complexity later --- revel/test.go | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/revel/test.go b/revel/test.go index 1358fd0b..a3f90542 100644 --- a/revel/test.go +++ b/revel/test.go @@ -326,3 +326,71 @@ func runTestSuites(baseURL, resultPath string, testSuites *[]controllers.TestSui } return &failedResultsSlice, len(failedResultsSlice) == 0 } + +func runTestSuitesSerial(baseURL, resultPath string, testSuites *[]controllers.TestSuiteDesc) (*[]controllers.TestSuiteResult, bool) { + // Load the result template, which we execute for each suite. + module, _ := revel.ModuleByName("testrunner") + TemplateLoader := revel.NewTemplateLoader([]string{filepath.Join(module.Path, "app", "views")}) + if err := TemplateLoader.Refresh(); err != nil { + errorf("Failed to compile templates: %s", err) + } + resultTemplate, err := TemplateLoader.Template("TestRunner/SuiteResult.html") + if err != nil { + errorf("Failed to load suite result template: %s", err) + } + + var ( + overallSuccess = true + failedResults []controllers.TestSuiteResult + ) + for _, suite := range *testSuites { + // Print the name of the suite we're running. + name := suite.Name + if len(name) > 22 { + name = name[:19] + "..." + } + fmt.Printf("%-22s", name) + + // Run every test. + startTime := time.Now() + suiteResult := controllers.TestSuiteResult{Name: suite.Name, Passed: true} + for _, test := range suite.Tests { + testURL := baseURL + "/@tests/" + suite.Name + "/" + test.Name + resp, err := http.Get(testURL) + if err != nil { + errorf("Failed to fetch test result at url %s: %s", testURL, err) + } + defer func() { + _ = resp.Body.Close() + }() + + var testResult controllers.TestResult + err = json.NewDecoder(resp.Body).Decode(&testResult) + if err == nil && !testResult.Passed { + suiteResult.Passed = false + } + suiteResult.Results = append(suiteResult.Results, testResult) + } + overallSuccess = overallSuccess && suiteResult.Passed + + // Print result. (Just PASSED or FAILED, and the time taken) + suiteResultStr, suiteAlert := "PASSED", "" + if !suiteResult.Passed { + suiteResultStr, suiteAlert = "FAILED", "!" + failedResults = append(failedResults, suiteResult) + } + fmt.Printf("%8s%3s%6ds\n", suiteResultStr, suiteAlert, int(time.Since(startTime).Seconds())) + // Create the result HTML file. + suiteResultFilename := filepath.Join(resultPath, + fmt.Sprintf("%s.%s.html", suite.Name, strings.ToLower(suiteResultStr))) + suiteResultFile, err := os.Create(suiteResultFilename) + if err != nil { + errorf("Failed to create result file %s: %s", suiteResultFilename, err) + } + if err = resultTemplate.Render(suiteResultFile, suiteResult); err != nil { + errorf("Failed to render result template: %s", err) + } + } + + return &failedResults, overallSuccess +} From 344dc63d22bccddc0e12f782d99d9ea86485f6a1 Mon Sep 17 00:00:00 2001 From: Eik Slowhand Madsen Date: Sun, 5 Apr 2020 18:53:13 +0200 Subject: [PATCH 5/7] - name the function --- harness/reflect.go | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/harness/reflect.go b/harness/reflect.go index 77d60077..f374a4f1 100644 --- a/harness/reflect.go +++ b/harness/reflect.go @@ -102,8 +102,7 @@ func ProcessSource(roots []string) (*SourceInfo, *revel.Error) { continue } - // Start walking the directory tree. - _ = revel.Walk(root, func(path string, info os.FileInfo, err error) error { + stroll := func(path string, info os.FileInfo, err error) error { if err != nil { log.Println("Error scanning app source:", err) return nil @@ -163,6 +162,15 @@ func ProcessSource(roots []string) (*SourceInfo, *revel.Error) { // There should be only one package in this directory. if len(pkgs) > 1 { log.Println("Most unexpected! Multiple packages in a single directory:", pkgs) + // filter out test packages + for k := range pkgs { + if strings.HasSuffix(k, "_test") { + delete(pkgs, k) + } + } + if len(pkgs) > 1 { + log.Fatalf("more than one non test package found in dir: %s", path) + } } var pkg *ast.Package @@ -172,7 +180,10 @@ func ProcessSource(roots []string) (*SourceInfo, *revel.Error) { srcInfo = appendSourceInfo(srcInfo, processPackage(fset, pkgImportPath, path, pkg)) return nil - }) + } + + // Start walking the directory tree. + _ = revel.Walk(root, stroll) } return srcInfo, compileError From 82d00b00df2a8e8d199dde02fbafe3c1e294175e Mon Sep 17 00:00:00 2001 From: Eik Slowhand Madsen Date: Sun, 5 Apr 2020 19:09:42 +0200 Subject: [PATCH 6/7] - cache the imports --- harness/import_cache.go | 122 ++++++++++++++++++++++++++++++++++++++++ harness/reflect.go | 108 +---------------------------------- 2 files changed, 124 insertions(+), 106 deletions(-) create mode 100644 harness/import_cache.go diff --git a/harness/import_cache.go b/harness/import_cache.go new file mode 100644 index 00000000..7df1f448 --- /dev/null +++ b/harness/import_cache.go @@ -0,0 +1,122 @@ +package harness + +import ( + "go/ast" + "go/build" + "go/token" + "strings" + + "github.com/revel/revel" +) + +type ImportCache map[string]string + +func (ip ImportCache) processPackage(fset *token.FileSet, pkgImportPath, pkgPath string, pkg *ast.Package) *SourceInfo { + var ( + structSpecs []*TypeInfo + initImportPaths []string + + methodSpecs = make(methodMap) + validationKeys = make(map[string]map[int]string) + scanControllers = strings.HasSuffix(pkgImportPath, "/controllers") || + strings.Contains(pkgImportPath, "/controllers/") + scanTests = strings.HasSuffix(pkgImportPath, "/tests") || + strings.Contains(pkgImportPath, "/tests/") + ) + + // For each source file in the package... + for _, file := range pkg.Files { + + // Imports maps the package key to the full import path. + // e.g. import "sample/app/models" => "models": "sample/app/models" + imports := map[string]string{} + + // For each declaration in the source file... + for _, decl := range file.Decls { + ip.addImports(imports, decl, pkgPath) + + if scanControllers { + // Match and add both structs and methods + structSpecs = appendStruct(structSpecs, pkgImportPath, pkg, decl, imports, fset) + appendAction(fset, methodSpecs, decl, pkgImportPath, pkg.Name, imports) + } else if scanTests { + structSpecs = appendStruct(structSpecs, pkgImportPath, pkg, decl, imports, fset) + } + + // If this is a func... + if funcDecl, ok := decl.(*ast.FuncDecl); ok { + // Scan it for validation calls + lineKeys := getValidationKeys(fset, funcDecl, imports) + if len(lineKeys) > 0 { + validationKeys[pkgImportPath+"."+getFuncName(funcDecl)] = lineKeys + } + + // Check if it's an init function. + if funcDecl.Name.Name == "init" { + initImportPaths = []string{pkgImportPath} + } + } + } + } + + // Add the method specs to the struct specs. + for _, spec := range structSpecs { + spec.MethodSpecs = methodSpecs[spec.StructName] + } + + return &SourceInfo{ + StructSpecs: structSpecs, + ValidationKeys: validationKeys, + InitImportPaths: initImportPaths, + } +} + +func (ip ImportCache) addImports(imports map[string]string, decl ast.Decl, srcDir string) { + genDecl, ok := decl.(*ast.GenDecl) + if !ok { + return + } + + if genDecl.Tok != token.IMPORT { + return + } + + for _, spec := range genDecl.Specs { + importSpec := spec.(*ast.ImportSpec) + var pkgAlias string + if importSpec.Name != nil { + pkgAlias = importSpec.Name.Name + if pkgAlias == "_" { + continue + } + } + quotedPath := importSpec.Path.Value // e.g. "\"sample/app/models\"" + fullPath := quotedPath[1 : len(quotedPath)-1] // Remove the quotes + + if pkgAlias == "" { + pkgAlias = ip[fullPath] + } + // If the package was not aliased (common case), we have to import it + // to see what the package name is. + // TODO: Can improve performance here a lot: + // 1. Do not import everything over and over again. Keep a cache. + // 2. Exempt the standard library; their directories always match the package name. + // 3. Can use build.FindOnly and then use parser.ParseDir with mode PackageClauseOnly + + if pkgAlias == "" { + pkg, err := build.Import(fullPath, srcDir, 0) + if err != nil { + // We expect this to happen for apps using reverse routing (since we + // have not yet generated the routes). Don't log that. + if !strings.HasSuffix(fullPath, "/app/routes") { + revel.TRACE.Println("Could not find import:", fullPath) + } + continue + } + pkgAlias = pkg.Name + ip[fullPath] = pkgAlias + } + + imports[pkgAlias] = fullPath + } +} diff --git a/harness/reflect.go b/harness/reflect.go index f374a4f1..7eec9c40 100644 --- a/harness/reflect.go +++ b/harness/reflect.go @@ -94,6 +94,7 @@ func ProcessSource(roots []string) (*SourceInfo, *revel.Error) { srcInfo *SourceInfo compileError *revel.Error ) + ip := ImportCache{} for _, root := range roots { rootImportPath := importPathFromPath(root) @@ -178,7 +179,7 @@ func ProcessSource(roots []string) (*SourceInfo, *revel.Error) { pkg = v } - srcInfo = appendSourceInfo(srcInfo, processPackage(fset, pkgImportPath, path, pkg)) + srcInfo = appendSourceInfo(srcInfo, ip.processPackage(fset, pkgImportPath, path, pkg)) return nil } @@ -206,66 +207,6 @@ func appendSourceInfo(srcInfo1, srcInfo2 *SourceInfo) *SourceInfo { return srcInfo1 } -func processPackage(fset *token.FileSet, pkgImportPath, pkgPath string, pkg *ast.Package) *SourceInfo { - var ( - structSpecs []*TypeInfo - initImportPaths []string - - methodSpecs = make(methodMap) - validationKeys = make(map[string]map[int]string) - scanControllers = strings.HasSuffix(pkgImportPath, "/controllers") || - strings.Contains(pkgImportPath, "/controllers/") - scanTests = strings.HasSuffix(pkgImportPath, "/tests") || - strings.Contains(pkgImportPath, "/tests/") - ) - - // For each source file in the package... - for _, file := range pkg.Files { - - // Imports maps the package key to the full import path. - // e.g. import "sample/app/models" => "models": "sample/app/models" - imports := map[string]string{} - - // For each declaration in the source file... - for _, decl := range file.Decls { - addImports(imports, decl, pkgPath) - - if scanControllers { - // Match and add both structs and methods - structSpecs = appendStruct(structSpecs, pkgImportPath, pkg, decl, imports, fset) - appendAction(fset, methodSpecs, decl, pkgImportPath, pkg.Name, imports) - } else if scanTests { - structSpecs = appendStruct(structSpecs, pkgImportPath, pkg, decl, imports, fset) - } - - // If this is a func... - if funcDecl, ok := decl.(*ast.FuncDecl); ok { - // Scan it for validation calls - lineKeys := getValidationKeys(fset, funcDecl, imports) - if len(lineKeys) > 0 { - validationKeys[pkgImportPath+"."+getFuncName(funcDecl)] = lineKeys - } - - // Check if it's an init function. - if funcDecl.Name.Name == "init" { - initImportPaths = []string{pkgImportPath} - } - } - } - } - - // Add the method specs to the struct specs. - for _, spec := range structSpecs { - spec.MethodSpecs = methodSpecs[spec.StructName] - } - - return &SourceInfo{ - StructSpecs: structSpecs, - ValidationKeys: validationKeys, - InitImportPaths: initImportPaths, - } -} - // getFuncName returns a name for this func or method declaration. // e.g. "(*Application).SayHello" for a method, "SayHello" for a func. func getFuncName(funcDecl *ast.FuncDecl) string { @@ -282,51 +223,6 @@ func getFuncName(funcDecl *ast.FuncDecl) string { return prefix + funcDecl.Name.Name } -func addImports(imports map[string]string, decl ast.Decl, srcDir string) { - genDecl, ok := decl.(*ast.GenDecl) - if !ok { - return - } - - if genDecl.Tok != token.IMPORT { - return - } - - for _, spec := range genDecl.Specs { - importSpec := spec.(*ast.ImportSpec) - var pkgAlias string - if importSpec.Name != nil { - pkgAlias = importSpec.Name.Name - if pkgAlias == "_" { - continue - } - } - quotedPath := importSpec.Path.Value // e.g. "\"sample/app/models\"" - fullPath := quotedPath[1 : len(quotedPath)-1] // Remove the quotes - - // If the package was not aliased (common case), we have to import it - // to see what the package name is. - // TODO: Can improve performance here a lot: - // 1. Do not import everything over and over again. Keep a cache. - // 2. Exempt the standard library; their directories always match the package name. - // 3. Can use build.FindOnly and then use parser.ParseDir with mode PackageClauseOnly - if pkgAlias == "" { - pkg, err := build.Import(fullPath, srcDir, 0) - if err != nil { - // We expect this to happen for apps using reverse routing (since we - // have not yet generated the routes). Don't log that. - if !strings.HasSuffix(fullPath, "/app/routes") { - revel.TRACE.Println("Could not find import:", fullPath) - } - continue - } - pkgAlias = pkg.Name - } - - imports[pkgAlias] = fullPath - } -} - // If this Decl is a struct type definition, it is summarized and added to specs. // Else, specs is returned unchanged. func appendStruct(specs []*TypeInfo, pkgImportPath string, pkg *ast.Package, decl ast.Decl, imports map[string]string, fset *token.FileSet) []*TypeInfo { From f55e27373084da3940746b50cd5795c0ddfebb31 Mon Sep 17 00:00:00 2001 From: Eik Slowhand Madsen Date: Sun, 5 Apr 2020 19:25:45 +0200 Subject: [PATCH 7/7] - update warning message --- harness/import_cache.go | 1 + harness/reflect.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/harness/import_cache.go b/harness/import_cache.go index 7df1f448..de4f13e1 100644 --- a/harness/import_cache.go +++ b/harness/import_cache.go @@ -96,6 +96,7 @@ func (ip ImportCache) addImports(imports map[string]string, decl ast.Decl, srcDi if pkgAlias == "" { pkgAlias = ip[fullPath] } + // If the package was not aliased (common case), we have to import it // to see what the package name is. // TODO: Can improve performance here a lot: diff --git a/harness/reflect.go b/harness/reflect.go index 7eec9c40..b9b522ad 100644 --- a/harness/reflect.go +++ b/harness/reflect.go @@ -162,7 +162,7 @@ func ProcessSource(roots []string) (*SourceInfo, *revel.Error) { // There should be only one package in this directory. if len(pkgs) > 1 { - log.Println("Most unexpected! Multiple packages in a single directory:", pkgs) + log.Println("Most unexpected! Multiple packages in a single directory, removing *_test packages:", pkgs) // filter out test packages for k := range pkgs { if strings.HasSuffix(k, "_test") {