From a5ca8fc6783e8b9cf36f05503b974e381299c674 Mon Sep 17 00:00:00 2001 From: SmallCoccinelle <89733524+SmallCoccinelle@users.noreply.github.com> Date: Sun, 3 Oct 2021 02:48:03 +0200 Subject: [PATCH] Enable safe linters (#1786) * Enable safe linters Enable the linters dogsled, rowserrcheck, and sqlclosecheck. These report no errors currently in the code base. Enable misspell. Misspell finds two spelling mistakes in comments, which are fixed by the patch as well. Add and sort linters which are relatively safe to add over time. Comment them out for now. * Close the response body If we can get a HTTP response, it has a body which ought to be closed. By doing so, we avoid potentially leaking connections. * Enable the exportloopref linter There are two places in the code with these warnings. Fix them while enabling the linter. * Remove redundant types in tests If a slice already determines the type, the inner type declaration is redundant. Remove the inner declarations. * Mark autotag test cases as parallel Autotag test cases is by far the outlier when it comes to test time. While go test runs test cases in parallel, it doesn't do so inside a given package, unless one marks the test cases as parallel. This change provides a significant speedup on a 8-core machine for test runs. --- .golangci.yml | 19 +++++++++++++++++-- pkg/autotag/gallery_test.go | 6 ++++++ pkg/autotag/image_test.go | 6 ++++++ pkg/autotag/performer_test.go | 6 ++++++ pkg/autotag/scene_test.go | 6 ++++++ pkg/autotag/studio_test.go | 6 ++++++ pkg/autotag/tag_test.go | 6 ++++++ pkg/logger/plugin.go | 1 + pkg/manager/manager_tasks.go | 2 +- pkg/movie/export_test.go | 12 ++++++------ pkg/performer/export_test.go | 6 +++--- pkg/plugin/args.go | 1 + pkg/scraper/url.go | 1 + pkg/scraper/xpath_test.go | 18 +++++++++--------- pkg/studio/export_test.go | 12 ++++++------ 15 files changed, 81 insertions(+), 27 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index ffd332dcc..ca296c1dd 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -18,8 +18,24 @@ linters: - unused - varcheck # Linters added by the stash project + # - bodyclose + - dogsled + # - errorlint + # - exhaustive + - exportloopref + # - goconst + # - gocritic + # - goerr113 - gofmt + # - gosec + # - ifshort + - misspell + # - nakedret + # - noctx + # - paralleltest - revive + - rowserrcheck + - sqlclosecheck linters-settings: gofmt: @@ -63,5 +79,4 @@ linters-settings: - name: unused-parameter disabled: true - name: unreachable-code - - name: redefines-builtin-id - + - name: redefines-builtin-id \ No newline at end of file diff --git a/pkg/autotag/gallery_test.go b/pkg/autotag/gallery_test.go index 63d82a8d1..f2f498519 100644 --- a/pkg/autotag/gallery_test.go +++ b/pkg/autotag/gallery_test.go @@ -12,6 +12,8 @@ import ( const galleryExt = "zip" func TestGalleryPerformers(t *testing.T) { + t.Parallel() + const galleryID = 1 const performerName = "performer name" const performerID = 2 @@ -55,6 +57,8 @@ func TestGalleryPerformers(t *testing.T) { } func TestGalleryStudios(t *testing.T) { + t.Parallel() + const galleryID = 1 const studioName = "studio name" const studioID = 2 @@ -124,6 +128,8 @@ func TestGalleryStudios(t *testing.T) { } func TestGalleryTags(t *testing.T) { + t.Parallel() + const galleryID = 1 const tagName = "tag name" const tagID = 2 diff --git a/pkg/autotag/image_test.go b/pkg/autotag/image_test.go index c56494602..68b33ab74 100644 --- a/pkg/autotag/image_test.go +++ b/pkg/autotag/image_test.go @@ -12,6 +12,8 @@ import ( const imageExt = "jpg" func TestImagePerformers(t *testing.T) { + t.Parallel() + const imageID = 1 const performerName = "performer name" const performerID = 2 @@ -55,6 +57,8 @@ func TestImagePerformers(t *testing.T) { } func TestImageStudios(t *testing.T) { + t.Parallel() + const imageID = 1 const studioName = "studio name" const studioID = 2 @@ -124,6 +128,8 @@ func TestImageStudios(t *testing.T) { } func TestImageTags(t *testing.T) { + t.Parallel() + const imageID = 1 const tagName = "tag name" const tagID = 2 diff --git a/pkg/autotag/performer_test.go b/pkg/autotag/performer_test.go index 6f23baabe..3e6714ccd 100644 --- a/pkg/autotag/performer_test.go +++ b/pkg/autotag/performer_test.go @@ -9,6 +9,8 @@ import ( ) func TestPerformerScenes(t *testing.T) { + t.Parallel() + type test struct { performerName string expectedRegex string @@ -85,6 +87,8 @@ func testPerformerScenes(t *testing.T, performerName, expectedRegex string) { } func TestPerformerImages(t *testing.T) { + t.Parallel() + type test struct { performerName string expectedRegex string @@ -157,6 +161,8 @@ func testPerformerImages(t *testing.T, performerName, expectedRegex string) { } func TestPerformerGalleries(t *testing.T) { + t.Parallel() + type test struct { performerName string expectedRegex string diff --git a/pkg/autotag/scene_test.go b/pkg/autotag/scene_test.go index 5afb4fa5f..5e5b88806 100644 --- a/pkg/autotag/scene_test.go +++ b/pkg/autotag/scene_test.go @@ -145,6 +145,8 @@ func generateTestTable(testName, ext string) []pathTestTable { } func TestScenePerformers(t *testing.T) { + t.Parallel() + const sceneID = 1 const performerName = "performer name" const performerID = 2 @@ -188,6 +190,8 @@ func TestScenePerformers(t *testing.T) { } func TestSceneStudios(t *testing.T) { + t.Parallel() + const sceneID = 1 const studioName = "studio name" const studioID = 2 @@ -257,6 +261,8 @@ func TestSceneStudios(t *testing.T) { } func TestSceneTags(t *testing.T) { + t.Parallel() + const sceneID = 1 const tagName = "tag name" const tagID = 2 diff --git a/pkg/autotag/studio_test.go b/pkg/autotag/studio_test.go index a172c9360..f8c2df49e 100644 --- a/pkg/autotag/studio_test.go +++ b/pkg/autotag/studio_test.go @@ -55,6 +55,8 @@ var testStudioCases = []testStudioCase{ } func TestStudioScenes(t *testing.T) { + t.Parallel() + for _, p := range testStudioCases { testStudioScenes(t, p) } @@ -145,6 +147,8 @@ func testStudioScenes(t *testing.T, tc testStudioCase) { } func TestStudioImages(t *testing.T) { + t.Parallel() + for _, p := range testStudioCases { testStudioImages(t, p) } @@ -234,6 +238,8 @@ func testStudioImages(t *testing.T, tc testStudioCase) { } func TestStudioGalleries(t *testing.T) { + t.Parallel() + for _, p := range testStudioCases { testStudioGalleries(t, p) } diff --git a/pkg/autotag/tag_test.go b/pkg/autotag/tag_test.go index b85c104c8..07a85856e 100644 --- a/pkg/autotag/tag_test.go +++ b/pkg/autotag/tag_test.go @@ -55,6 +55,8 @@ var testTagCases = []testTagCase{ } func TestTagScenes(t *testing.T) { + t.Parallel() + for _, p := range testTagCases { testTagScenes(t, p) } @@ -141,6 +143,8 @@ func testTagScenes(t *testing.T, tc testTagCase) { } func TestTagImages(t *testing.T) { + t.Parallel() + for _, p := range testTagCases { testTagImages(t, p) } @@ -226,6 +230,8 @@ func testTagImages(t *testing.T, tc testTagCase) { } func TestTagGalleries(t *testing.T) { + t.Parallel() + for _, p := range testTagCases { testTagGalleries(t, p) } diff --git a/pkg/logger/plugin.go b/pkg/logger/plugin.go index b2e266ea9..67e6b03e9 100644 --- a/pkg/logger/plugin.go +++ b/pkg/logger/plugin.go @@ -112,6 +112,7 @@ func DetectLogLevel(line string) (*PluginLogLevel, string) { var level *PluginLogLevel for _, l := range validLevels { if l.char == char { + l := l // Make a copy of the loop variable level = &l break } diff --git a/pkg/manager/manager_tasks.go b/pkg/manager/manager_tasks.go index 345c06f18..27f341428 100644 --- a/pkg/manager/manager_tasks.go +++ b/pkg/manager/manager_tasks.go @@ -251,7 +251,7 @@ func (s *singleton) Generate(ctx context.Context, input models.GenerateMetadataI // Start measuring how long the generate has taken. (consider moving this up) start := time.Now() if err = instance.Paths.Generated.EnsureTmpDir(); err != nil { - logger.Warnf("could not create temprary directory: %v", err) + logger.Warnf("could not create temporary directory: %v", err) } for _, scene := range scenes { diff --git a/pkg/movie/export_test.go b/pkg/movie/export_test.go index 12023bb52..b03de426c 100644 --- a/pkg/movie/export_test.go +++ b/pkg/movie/export_test.go @@ -148,32 +148,32 @@ var scenarios []testScenario func initTestTable() { scenarios = []testScenario{ - testScenario{ + { createFullMovie(movieID, studioID), createFullJSONMovie(studioName, frontImage, backImage), false, }, - testScenario{ + { createEmptyMovie(emptyID), createEmptyJSONMovie(), false, }, - testScenario{ + { createFullMovie(errFrontImageID, studioID), nil, true, }, - testScenario{ + { createFullMovie(errBackImageID, studioID), nil, true, }, - testScenario{ + { createFullMovie(errStudioMovieID, errStudioID), nil, true, }, - testScenario{ + { createFullMovie(missingStudioMovieID, missingStudioID), createFullJSONMovie("", frontImage, backImage), false, diff --git a/pkg/performer/export_test.go b/pkg/performer/export_test.go index f8fe706bb..0ec00a93b 100644 --- a/pkg/performer/export_test.go +++ b/pkg/performer/export_test.go @@ -168,17 +168,17 @@ var scenarios []testScenario func initTestTable() { scenarios = []testScenario{ - testScenario{ + { *createFullPerformer(performerID, performerName), createFullJSONPerformer(performerName, image), false, }, - testScenario{ + { createEmptyPerformer(noImageID), createEmptyJSONPerformer(), false, }, - testScenario{ + { *createFullPerformer(errImageID, performerName), nil, true, diff --git a/pkg/plugin/args.go b/pkg/plugin/args.go index 13fc3570e..adcdf007f 100644 --- a/pkg/plugin/args.go +++ b/pkg/plugin/args.go @@ -17,6 +17,7 @@ func findArg(args []*models.PluginArgInput, name string) *models.PluginArgInput func applyDefaultArgs(args []*models.PluginArgInput, defaultArgs map[string]string) []*models.PluginArgInput { for k, v := range defaultArgs { if arg := findArg(args, k); arg == nil { + v := v // Copy v, because it's being exported out of the loop args = append(args, &models.PluginArgInput{ Key: k, Value: &models.PluginValueInput{ diff --git a/pkg/scraper/url.go b/pkg/scraper/url.go index a8f60ff15..fe33c157d 100644 --- a/pkg/scraper/url.go +++ b/pkg/scraper/url.go @@ -223,6 +223,7 @@ func getRemoteCDPWSAddress(address string) (string, error) { if err != nil { return "", err } + defer resp.Body.Close() var result map[string]interface{} var json = jsoniter.ConfigCompatibleWithStandardLibrary diff --git a/pkg/scraper/xpath_test.go b/pkg/scraper/xpath_test.go index 60fb5749f..4ba98d40d 100644 --- a/pkg/scraper/xpath_test.go +++ b/pkg/scraper/xpath_test.go @@ -53,9 +53,9 @@ const htmlDoc1 = ` Country of Origin: - + - + United States @@ -661,7 +661,7 @@ func verifyPerformers(t *testing.T, expectedNames []string, expectedURLs []strin t.Errorf("Expected performer name %s, got %s", expectedName, actualName) } if expectedURL != actualURL { - t.Errorf("Expected perfromer URL %s, got %s", expectedName, actualName) + t.Errorf("Expected performer URL %s, got %s", expectedName, actualName) } i++ } @@ -741,7 +741,7 @@ func TestLoadXPathScraperFromYAML(t *testing.T) { const yamlStr = `name: Test performerByURL: - action: scrapeXPath - url: + url: - test.com scraper: performerScraper xPathScrapers: @@ -755,11 +755,11 @@ xPathScrapers: postProcess: - parseDate: January 2, 2006 Tags: - Name: //tags + Name: //tags Movies: - Name: //movies + Name: //movies Performers: - Name: //performers + Name: //performers Studio: Name: //studio ` @@ -848,13 +848,13 @@ func TestSubScrape(t *testing.T) { yamlStr := `name: Test performerByURL: - action: scrapeXPath - url: + url: - ` + ts.URL + ` scraper: performerScraper xPathScrapers: performerScraper: performer: - Name: + Name: selector: //div/a/@href postProcess: - replace: diff --git a/pkg/studio/export_test.go b/pkg/studio/export_test.go index 1e8b2e88b..e251ad52c 100644 --- a/pkg/studio/export_test.go +++ b/pkg/studio/export_test.go @@ -119,32 +119,32 @@ var scenarios []testScenario func initTestTable() { scenarios = []testScenario{ - testScenario{ + { createFullStudio(studioID, parentStudioID), createFullJSONStudio(parentStudioName, image, []string{"alias"}), false, }, - testScenario{ + { createEmptyStudio(noImageID), createEmptyJSONStudio(), false, }, - testScenario{ + { createFullStudio(errImageID, parentStudioID), nil, true, }, - testScenario{ + { createFullStudio(missingParentStudioID, missingStudioID), createFullJSONStudio("", image, nil), false, }, - testScenario{ + { createFullStudio(errStudioID, errParentStudioID), nil, true, }, - testScenario{ + { createFullStudio(errAliasID, parentStudioID), nil, true,