Address review feedback

Signed-off-by: Alexander Bezzubov <bzz@apache.org>

common.go

@@ -16,7 +16,7 @@ const OtherLanguage = ""
 // Strategy type fix the signature for the functions that can be used as a strategy.
 type Strategy func(filename string, content []byte, candidates []string) (languages []string)
 
-// DefaultStrategies is a sequence of stratagies used by GetLanguage to detect languages.
+// DefaultStrategies is a sequence of strategies used by GetLanguage to detect languages.
 var DefaultStrategies = []Strategy{
 	GetLanguagesByModeline,
 	GetLanguagesByFilename,

internal/code-generator/generator/generator.go

@@ -1,5 +1,5 @@
-// Package generator provides facilities to generate Golang code from data in
-// Linguist.
+// Package generator provides facilities to generate Go code for the
+// package data in enry from YAML files describing supported languages in Linguist.
 package generator
 
 import (

internal/code-generator/generator/generator_test.go

@@ -1,6 +1,7 @@
 package generator
 
 import (
+	"flag"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -99,11 +100,13 @@ type testCase struct {
 	wantOut     string
 }
 
+var updateGold = flag.Bool("update_gold", false, "Update golden test files")
+
 func Test_GeneratorTestSuite(t *testing.T) {
 	suite.Run(t, new(GeneratorTestSuite))
 }
 
-func (s *GeneratorTestSuite) cloneLinguistMaybe() {
+func (s *GeneratorTestSuite) maybeCloneLinguist() {
 	var err error
 	s.tmpLinguist = os.Getenv(linguistClonedEnvVar)
 	s.cloned = s.tmpLinguist == ""
@@ -130,7 +133,7 @@ func (s *GeneratorTestSuite) cloneLinguistMaybe() {
 }
 
 func (s *GeneratorTestSuite) SetupSuite() {
-	s.cloneLinguistMaybe()
+	s.maybeCloneLinguist()
 	s.testCases = []testCase{
 		{
 			name:        "Extensions()",
@@ -246,14 +249,19 @@ func (s *GeneratorTestSuite) SetupSuite() {
 func (s *GeneratorTestSuite) TearDownSuite() {
 	if s.cloned {
 		err := os.RemoveAll(s.tmpLinguist)
-		assert.NoError(s.T(), err)
+		if err != nil {
+			s.T().Logf("Failed to clean up %s after the test.\n", s.tmpLinguist)
+		}
 	}
 }
 
-// UpdateGeneratorTestSuiteGold is a Gold results generation automation.
-// It should only be enabled&run manually on every new linuguist verision
-// to update *.gold.
-func (s *GeneratorTestSuite) /*Test*/ UpdateGeneratorTestSuiteGold() {
+// TestUpdateGeneratorTestSuiteGold is a Gold results generation automation.
+// It should only be enabled&run manually on every new Linguist version
+// to update *.gold files.
+func (s *GeneratorTestSuite) TestUpdateGeneratorTestSuiteGold() {
+	if !*updateGold {
+		s.T().Skip()
+	}
 	for _, test := range s.testCases {
 		dst := test.wantOut
 		s.T().Logf("Generating %s from %s\n", dst, test.fileToParse)

internal/code-generator/generator/heuristics_test.go

@@ -11,7 +11,7 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestYamlParsing(t *testing.T) {
+func TestYAMLParsing(t *testing.T) {
 	heuristics, err := parseYaml("test_files/heuristics.yml")
 
 	require.NoError(t, err)
@@ -45,10 +45,6 @@ func TestYamlParsing(t *testing.T) {
 	assert.Equal(t, "Linux Kernel Module", rules[0].Languages[0])
 	assert.Equal(t, "AMPL", rules[0].Languages[1])
 
-	// for i, rule := range rules {
-	// 	t.Logf("\n\t Forth rule%d: %q\n", i, rule)
-	// }
-
 	// named_patterns
 	require.NotNil(t, heuristics.NamedPatterns)
 	assert.Equal(t, 2, len(heuristics.NamedPatterns))
@@ -57,19 +53,18 @@ func TestYamlParsing(t *testing.T) {
 }
 
 func TestSingleRuleLoading(t *testing.T) {
-	//given
 	namedPatterns := map[string]StringArray{"cpp": []string{"cpp_ptrn1", "cpp_ptrn2"}}
 	rules := []*Rule{
 		&Rule{Languages: []string{"a"}, Patterns: Patterns{NamedPattern: "cpp"}},
 		&Rule{Languages: []string{"b"}, And: []*Rule{}},
 	}
 
-	//named_pattern
+	// named_pattern case
 	langPattern := loadRule(namedPatterns, rules[0])
 	require.Equal(t, "a", langPattern.Langs[0])
 	assert.NotEmpty(t, langPattern.Pattern)
 
-	//and
+	// and case
 	langPattern = loadRule(namedPatterns, rules[1])
 	require.Equal(t, "b", langPattern.Langs[0])
 }
@@ -109,7 +104,7 @@ func TestTemplateMatcherVars(t *testing.T) {
 	heuristics, err := loadHeuristics(parsed)
 	require.NoError(t, err)
 
-	//render tmpl
+	// render a tmpl
 	const contentTmpl = "../assets/content.go.tmpl"
 	tmpl, err := template.ParseFiles(contentTmpl)
 	require.NoError(t, err)
@@ -119,11 +114,10 @@ func TestTemplateMatcherVars(t *testing.T) {
 	require.NoError(t, err, fmt.Sprintf("%+v", tmpl))
 	require.NotEmpty(t, buf)
 
-	//TODO verify generated code:
-	//check key literal exists in map for each extension:
-	// pattern, negative_pattern,
+	// TODO(bzz) add more advanced test using go/ast package, to verify the
+	// strucutre of generated code:
+	//  - check key literal exists in map for each extension:
 
 	src, err := format.Source(buf.Bytes())
-	require.NoError(t, err, buf.String())
-	t.Logf("\n%s\n", string(src))
+	require.NoError(t, err, "\n%s\n", string(src))
 }

internal/code-generator/generator/test_files/heuristics.yml

@@ -1,3 +1,4 @@
+# Tests care about number and order of heuristics in this fixture
 disambiguations:
 
 - extensions: ['.h', '.hh']