revel/revel#1057 code improvements docs, errcheck, cyclo, etc

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 (

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 (

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 (

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.

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 (

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{}{

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 (

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,

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))

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, " ")

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]
 	}

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
+}

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
 }

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 (