From a5c8a8da092cc2794ec0540abc8d5609553fe483 Mon Sep 17 00:00:00 2001 From: Jeevanandam M Date: Thu, 9 Jun 2016 10:33:02 -0700 Subject: [PATCH] revel/revel#1057 code improvements docs, errcheck, cyclo, etc --- harness/app.go | 4 + harness/build.go | 4 + harness/harness.go | 9 +- harness/reflect.go | 4 + harness/reflect_test.go | 4 + revel/build.go | 23 ++-- revel/clean.go | 4 + revel/new.go | 6 +- revel/package.go | 10 +- revel/rev.go | 8 ++ revel/run.go | 6 +- revel/test.go | 251 ++++++++++++++++++++++------------------ revel/util.go | 28 +++-- revel/version.go | 4 + 14 files changed, 233 insertions(+), 132 deletions(-) diff --git a/harness/app.go b/harness/app.go index 63e0060..c1b142c 100644 --- a/harness/app.go +++ b/harness/app.go @@ -1,3 +1,7 @@ +// Copyright (c) 2012-2016 The Revel Framework Authors, All rights reserved. +// Revel Framework source code and usage is governed by a MIT style +// license that can be found in the LICENSE file. + package harness import ( diff --git a/harness/build.go b/harness/build.go index 4b4d5a1..4e9c672 100755 --- a/harness/build.go +++ b/harness/build.go @@ -1,3 +1,7 @@ +// Copyright (c) 2012-2016 The Revel Framework Authors, All rights reserved. +// Revel Framework source code and usage is governed by a MIT style +// license that can be found in the LICENSE file. + package harness import ( diff --git a/harness/harness.go b/harness/harness.go index 4d46713..ccb877e 100644 --- a/harness/harness.go +++ b/harness/harness.go @@ -1,13 +1,16 @@ -// The Harness for a Revel program. +// Copyright (c) 2012-2016 The Revel Framework Authors, All rights reserved. +// Revel Framework source code and usage is governed by a MIT style +// license that can be found in the LICENSE file. + +// Package harness for a Revel Framework. // -// It has a couple responsibilities: +// It has a following responsibilities: // 1. Parse the user program, generating a main.go file that registers // controller classes and starts the user's server. // 2. Build and run the user program. Show compile errors. // 3. Monitor the user source and re-build / restart the program when necessary. // // Source files are generated in the app/tmp directory. - package harness import ( diff --git a/harness/reflect.go b/harness/reflect.go index 853f76c..ded4b7d 100644 --- a/harness/reflect.go +++ b/harness/reflect.go @@ -1,3 +1,7 @@ +// Copyright (c) 2012-2016 The Revel Framework Authors, All rights reserved. +// Revel Framework source code and usage is governed by a MIT style +// license that can be found in the LICENSE file. + package harness // This file handles the app code introspection. diff --git a/harness/reflect_test.go b/harness/reflect_test.go index 9815ca2..9db9b23 100644 --- a/harness/reflect_test.go +++ b/harness/reflect_test.go @@ -1,3 +1,7 @@ +// Copyright (c) 2012-2016 The Revel Framework Authors, All rights reserved. +// Revel Framework source code and usage is governed by a MIT style +// license that can be found in the LICENSE file. + package harness import ( diff --git a/revel/build.go b/revel/build.go index 25d2524..58ee4e3 100644 --- a/revel/build.go +++ b/revel/build.go @@ -1,3 +1,7 @@ +// Copyright (c) 2012-2016 The Revel Framework Authors, All rights reserved. +// Revel Framework source code and usage is governed by a MIT style +// license that can be found in the LICENSE file. + package main import ( @@ -40,7 +44,7 @@ func buildApp(args []string) { return } - appImportPath, destPath, mode := args[0], args[1], "dev" + appImportPath, destPath, mode := args[0], args[1], DefaultRunMode if len(args) >= 3 { mode = args[2] } @@ -55,8 +59,13 @@ func buildApp(args []string) { errorf("Abort: %s exists and does not look like a build directory.", destPath) } - os.RemoveAll(destPath) - os.MkdirAll(destPath, 0777) + if err := os.RemoveAll(destPath); err != nil && !os.IsNotExist(err) { + revel.ERROR.Fatalln(err) + } + + if err := os.MkdirAll(destPath, 0777); err != nil { + revel.ERROR.Fatalln(err) + } app, reverr := harness.Build() panicOnError(reverr, "Failed to build") @@ -73,9 +82,9 @@ func buildApp(args []string) { tmpRevelPath := filepath.Join(srcPath, filepath.FromSlash(revel.RevelImportPath)) mustCopyFile(destBinaryPath, app.BinaryPath) mustChmod(destBinaryPath, 0755) - mustCopyDir(filepath.Join(tmpRevelPath, "conf"), filepath.Join(revel.RevelPath, "conf"), nil) - mustCopyDir(filepath.Join(tmpRevelPath, "templates"), filepath.Join(revel.RevelPath, "templates"), nil) - mustCopyDir(filepath.Join(srcPath, filepath.FromSlash(appImportPath)), revel.BasePath, nil) + _ = mustCopyDir(filepath.Join(tmpRevelPath, "conf"), filepath.Join(revel.RevelPath, "conf"), nil) + _ = mustCopyDir(filepath.Join(tmpRevelPath, "templates"), filepath.Join(revel.RevelPath, "templates"), nil) + _ = mustCopyDir(filepath.Join(srcPath, filepath.FromSlash(appImportPath)), revel.BasePath, nil) // Find all the modules used and copy them over. config := revel.Config.Raw() @@ -98,7 +107,7 @@ func buildApp(args []string) { } } for importPath, fsPath := range modulePaths { - mustCopyDir(filepath.Join(srcPath, importPath), fsPath, nil) + _ = mustCopyDir(filepath.Join(srcPath, importPath), fsPath, nil) } tmplData, runShPath := map[string]interface{}{ diff --git a/revel/clean.go b/revel/clean.go index 254b549..0a0126a 100644 --- a/revel/clean.go +++ b/revel/clean.go @@ -1,3 +1,7 @@ +// Copyright (c) 2012-2016 The Revel Framework Authors, All rights reserved. +// Revel Framework source code and usage is governed by a MIT style +// license that can be found in the LICENSE file. + package main import ( diff --git a/revel/new.go b/revel/new.go index 1ada576..306672d 100644 --- a/revel/new.go +++ b/revel/new.go @@ -1,3 +1,7 @@ +// Copyright (c) 2012-2016 The Revel Framework Authors, All rights reserved. +// Revel Framework source code and usage is governed by a MIT style +// license that can be found in the LICENSE file. + package main import ( @@ -199,7 +203,7 @@ func copyNewAppFiles() { err = os.MkdirAll(appPath, 0777) panicOnError(err, "Failed to create directory "+appPath) - mustCopyDir(appPath, skeletonPath, map[string]interface{}{ + _ = mustCopyDir(appPath, skeletonPath, map[string]interface{}{ // app.conf "AppName": appName, "BasePath": basePath, diff --git a/revel/package.go b/revel/package.go index 41ea785..0eb4d40 100644 --- a/revel/package.go +++ b/revel/package.go @@ -1,3 +1,7 @@ +// Copyright (c) 2012-2016 The Revel Framework Authors, All rights reserved. +// Revel Framework source code and usage is governed by a MIT style +// license that can be found in the LICENSE file. + package main import ( @@ -38,7 +42,7 @@ func packageApp(args []string) { } // Determine the run mode. - mode := "dev" + mode := DefaultRunMode if len(args) >= 2 { mode = args[1] } @@ -48,7 +52,9 @@ func packageApp(args []string) { // Remove the archive if it already exists. destFile := filepath.Base(revel.BasePath) + ".tar.gz" - os.Remove(destFile) + if err := os.Remove(destFile); err != nil && !os.IsNotExist(err) { + revel.ERROR.Fatal(err) + } // Collect stuff in a temp directory. tmpDir, err := ioutil.TempDir("", filepath.Base(revel.BasePath)) diff --git a/revel/rev.go b/revel/rev.go index 6c234b8..2691826 100644 --- a/revel/rev.go +++ b/revel/rev.go @@ -1,3 +1,7 @@ +// Copyright (c) 2012-2016 The Revel Framework Authors, All rights reserved. +// Revel Framework source code and usage is governed by a MIT style +// license that can be found in the LICENSE file. + // The command line tool for running Revel apps. package main @@ -15,12 +19,16 @@ import ( "github.com/agtorre/gocolorize" ) +// DefaultRunMode for revel's application +const DefaultRunMode = "dev" + // Command structure cribbed from the genius organization of the "go" command. type Command struct { Run func(args []string) UsageLine, Short, Long string } +// Name returns command name from usage line func (cmd *Command) Name() string { name := cmd.UsageLine i := strings.Index(name, " ") diff --git a/revel/run.go b/revel/run.go index 4d4572f..22f07d0 100644 --- a/revel/run.go +++ b/revel/run.go @@ -1,3 +1,7 @@ +// Copyright (c) 2012-2016 The Revel Framework Authors, All rights reserved. +// Revel Framework source code and usage is governed by a MIT style +// license that can be found in the LICENSE file. + package main import ( @@ -37,7 +41,7 @@ func runApp(args []string) { } // Determine the run mode. - mode := "dev" + mode := DefaultRunMode if len(args) >= 2 { mode = args[1] } diff --git a/revel/test.go b/revel/test.go index ac01bc2..10d5bac 100644 --- a/revel/test.go +++ b/revel/test.go @@ -1,3 +1,7 @@ +// Copyright (c) 2012-2016 The Revel Framework Authors, All rights reserved. +// Revel Framework source code and usage is governed by a MIT style +// license that can be found in the LICENSE file. + package main import ( @@ -52,7 +56,7 @@ func testApp(args []string) { errorf("No import path given.\nRun 'revel help test' for usage.\n") } - mode := "dev" + mode := DefaultRunMode if len(args) >= 2 { mode = args[1] } @@ -61,22 +65,7 @@ func testApp(args []string) { revel.Init(mode, args[0], "") // Ensure that the testrunner is loaded in this mode. - testRunnerFound := false - for _, module := range revel.Modules { - if module.ImportPath == revel.Config.StringDefault("module.testrunner", "github.com/revel/modules/testrunner") { - testRunnerFound = true - break - } - } - if !testRunnerFound { - errorf(`Error: The testrunner module is not running. - -You can add it to a run mode configuration with the following line: - - module.testrunner = github.com/revel/modules/testrunner - -`) - } + checkTestRunner() // Create a directory to hold the test result files. resultPath := filepath.Join(revel.BasePath, "test-results") @@ -108,109 +97,27 @@ You can add it to a run mode configuration with the following line: defer cmd.Kill() revel.INFO.Printf("Testing %s (%s) in %s mode\n", revel.AppName, revel.ImportPath, mode) - // Get a list of tests. - // Since this is the first request to the server, retry/sleep a couple times - // in case it hasn't finished starting up yet. - var ( - testSuites []controllers.TestSuiteDesc - resp *http.Response - baseUrl = fmt.Sprintf("http://127.0.0.1:%d", revel.HTTPPort) - ) - for i := 0; ; i++ { - if resp, err = http.Get(baseUrl + "/@tests.list"); err == nil { - if resp.StatusCode == http.StatusOK { - break - } - } - if i < 3 { - time.Sleep(3 * time.Second) - continue - } - if err != nil { - errorf("Failed to request test list: %s", err) - } else { - errorf("Failed to request test list: non-200 response") - } - } - defer resp.Body.Close() - json.NewDecoder(resp.Body).Decode(&testSuites) + // Get a list of tests + var baseURL = fmt.Sprintf("http://127.0.0.1:%d", revel.HTTPPort) + testSuites, _ := getTestsList(baseURL) // If a specific TestSuite[.Method] is specified, only run that suite/test if len(args) == 3 { testSuites = filterTestSuites(testSuites, args[2]) } - fmt.Printf("\n%d test suite%s to run.\n", len(testSuites), pluralize(len(testSuites), "", "s")) + testSuiteCount := len(*testSuites) + fmt.Printf("\n%d test suite%s to run.\n", testSuiteCount, pluralize(testSuiteCount, "", "s")) fmt.Println() - // 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) - } - // Run each suite. - 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 resp.Body.Close() - - var testResult controllers.TestResult - json.NewDecoder(resp.Body).Decode(&testResult) - if !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) - } - } + failedResults, overallSuccess := runTestSuites(baseURL, resultPath, testSuites) fmt.Println() if overallSuccess { writeResultFile(resultPath, "result.passed", "passed") fmt.Println("All Tests Passed.") } else { - for _, failedResult := range failedResults { + for _, failedResult := range *failedResults { fmt.Printf("Failures:\n") for _, result := range failedResult.Results { if !result.Passed { @@ -239,7 +146,7 @@ func pluralize(num int, singular, plural string) string { // Filters test suites and individual tests to match // the parsed command line parameter -func filterTestSuites(suites []controllers.TestSuiteDesc, suiteArgument string) []controllers.TestSuiteDesc { +func filterTestSuites(suites *[]controllers.TestSuiteDesc, suiteArgument string) *[]controllers.TestSuiteDesc { var suiteName, testName string argArray := strings.Split(suiteArgument, ".") suiteName = argArray[0] @@ -249,20 +156,20 @@ func filterTestSuites(suites []controllers.TestSuiteDesc, suiteArgument string) if len(argArray) == 2 { testName = argArray[1] } - for _, suite := range suites { + for _, suite := range *suites { if suite.Name != suiteName { continue } if testName == "" { - return []controllers.TestSuiteDesc{suite} + return &[]controllers.TestSuiteDesc{suite} } // Only run a particular test in a suite for _, test := range suite.Tests { if test.Name != testName { continue } - return []controllers.TestSuiteDesc{ - controllers.TestSuiteDesc{ + return &[]controllers.TestSuiteDesc{ + { Name: suite.Name, Tests: []controllers.TestDesc{test}, }, @@ -273,3 +180,125 @@ func filterTestSuites(suites []controllers.TestSuiteDesc, suiteArgument string) errorf("Couldn't find test suite %s", suiteName) return nil } + +func checkTestRunner() { + testRunnerFound := false + for _, module := range revel.Modules { + if module.ImportPath == revel.Config.StringDefault("module.testrunner", "github.com/revel/modules/testrunner") { + testRunnerFound = true + break + } + } + + if !testRunnerFound { + errorf(`Error: The testrunner module is not running. + +You can add it to a run mode configuration with the following line: + + module.testrunner = github.com/revel/modules/testrunner + +`) + } +} + +// Get a list of tests from server. +// Since this is the first request to the server, retry/sleep a couple times +// in case it hasn't finished starting up yet. +func getTestsList(baseURL string) (*[]controllers.TestSuiteDesc, error) { + var ( + err error + resp *http.Response + testSuites []controllers.TestSuiteDesc + ) + for i := 0; ; i++ { + if resp, err = http.Get(baseURL + "/@tests.list"); err == nil { + if resp.StatusCode == http.StatusOK { + break + } + } + if i < 3 { + time.Sleep(3 * time.Second) + continue + } + if err != nil { + errorf("Failed to request test list: %s", err) + } else { + errorf("Failed to request test list: non-200 response") + } + } + defer func() { + _ = resp.Body.Close() + }() + + err = json.NewDecoder(resp.Body).Decode(&testSuites) + + return &testSuites, err +} + +func runTestSuites(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 +} diff --git a/revel/util.go b/revel/util.go index b15fd7c..9535cb6 100644 --- a/revel/util.go +++ b/revel/util.go @@ -1,3 +1,7 @@ +// Copyright (c) 2012-2016 The Revel Framework Authors, All rights reserved. +// Revel Framework source code and usage is governed by a MIT style +// license that can be found in the LICENSE file. + package main import ( @@ -13,7 +17,7 @@ import ( "github.com/revel/revel" ) -// Use a wrapper to differentiate logged panics from unexpected ones. +// LoggedError is wrapper to differentiate logged panics from unexpected ones. type LoggedError struct{ error } func panicOnError(err error, msg string) { @@ -103,22 +107,30 @@ func mustCopyDir(destDir, srcDir string, data map[string]interface{}) error { func mustTarGzDir(destFilename, srcDir string) string { zipFile, err := os.Create(destFilename) panicOnError(err, "Failed to create archive") - defer zipFile.Close() + defer func() { + _ = zipFile.Close() + }() gzipWriter := gzip.NewWriter(zipFile) - defer gzipWriter.Close() + defer func() { + _ = gzipWriter.Close() + }() tarWriter := tar.NewWriter(gzipWriter) - defer tarWriter.Close() + defer func() { + _ = tarWriter.Close() + }() - revel.Walk(srcDir, func(srcPath string, info os.FileInfo, err error) error { + _ = revel.Walk(srcDir, func(srcPath string, info os.FileInfo, err error) error { if info.IsDir() { return nil } srcFile, err := os.Open(srcPath) panicOnError(err, "Failed to read source file") - defer srcFile.Close() + defer func() { + _ = srcFile.Close() + }() err = tarWriter.WriteHeader(&tar.Header{ Name: strings.TrimLeft(srcPath[len(srcDir):], string(os.PathSeparator)), @@ -149,7 +161,9 @@ func empty(dirname string) bool { if err != nil { errorf("error opening directory: %s", err) } - defer dir.Close() + defer func() { + _ = dir.Close() + }() results, _ := dir.Readdir(1) return len(results) == 0 } diff --git a/revel/version.go b/revel/version.go index 7343b1c..c6ee21a 100644 --- a/revel/version.go +++ b/revel/version.go @@ -2,6 +2,10 @@ // Revel Framework source code and usage is governed by a MIT style // license that can be found in the LICENSE file. +// Copyright (c) 2012-2016 The Revel Framework Authors, All rights reserved. +// Revel Framework source code and usage is governed by a MIT style +// license that can be found in the LICENSE file. + package main import (