diff --git a/internal/file/finder_test.go b/internal/file/finder_test.go index 7bb52769..801a1436 100644 --- a/internal/file/finder_test.go +++ b/internal/file/finder_test.go @@ -6,7 +6,9 @@ import ( "io" "net/http" "os" + "path" "path/filepath" + "sort" "strings" "testing" @@ -97,8 +99,8 @@ func TestGetGroups(t *testing.T) { path := "" exclusions := []string{"testdata/go/*.mod", "testdata/misc/**"} - excludedFiles := []string{"testdata/go/go.mod", "testdata/misc/requirements.txt"} - const nbrOfGroups = 4 + excludedFiles := []string{"testdata/go/go.mod", "testdata/misc/requirements.txt", "testdata/misc/Cargo.lock"} + const nbrOfGroups = 5 fileGroups, err := finder.GetGroups(path, exclusions, false, StrictAll) @@ -119,6 +121,48 @@ func TestGetGroups(t *testing.T) { } } +func TestGetGroupsPIP(t *testing.T) { + setUp(true) + path := "testdata/pip" + const nbrOfGroups = 3 + + lockfileOnly := false + fileGroups, err := finder.GetGroups(path, []string{}, lockfileOnly, StrictAll) + + assert.NoError(t, err) + assert.Equalf(t, nbrOfGroups, fileGroups.Size(), "failed to assert that %d groups were created. %d was found", nbrOfGroups, fileGroups.Size()) + + locksFound := make([]string, 0) + manifestsFound := make([]string, 0) + for _, fileGroup := range fileGroups.ToSlice() { + lockFiles := fileGroup.LockFiles + locksFound = append(locksFound, lockFiles...) + manifestFile := fileGroup.ManifestFile + manifestsFound = append(manifestsFound, manifestFile) + } + manifestsExpected := []string{"testdata/pip/requirements-dev.txt", "testdata/pip/requirements.txt", "testdata/pip/requirements.test.txt"} + locksExpected := []string{"testdata/pip/requirements-dev.txt.pip.debricked.lock", "testdata/pip/.requirements.txt.pip.debricked.lock"} + sort.Strings(manifestsExpected) + sort.Strings(locksExpected) + sort.Strings(manifestsFound) + sort.Strings(locksFound) + t.Logf("manifest files expected: %s", manifestsExpected) + t.Logf("manifest files found: %s, len: %d", manifestsFound, len(manifestsFound)) + t.Logf("lock files expected: %s", manifestsExpected) + t.Logf("lock files found: %s, len: %d", locksFound, len(locksFound)) + for i := range manifestsExpected { + found := filepath.ToSlash(filepath.Clean(manifestsFound[i])) + expected := filepath.ToSlash(filepath.Clean(manifestsExpected[i])) + assert.Truef(t, found == expected, "Manifest files do not match! Found: %s | Expected: %s", found, expected) + } + for i := range locksExpected { + found := filepath.ToSlash(filepath.Clean(locksFound[i])) + expected := filepath.ToSlash(filepath.Clean(locksExpected[i])) + assert.Truef(t, found == expected, "Lock files do not match! Found: %s | Expected: %s", found, expected) + } + +} + func TestExclude(t *testing.T) { var files []string _ = filepath.Walk(".", @@ -182,7 +226,7 @@ func TestExclude(t *testing.T) { } } - assert.Equal(t, len(c.expectedExclusions), len(excludedFiles), "failed to assert that the same number of files were ignored") + assert.GreaterOrEqual(t, len(excludedFiles), len(c.expectedExclusions), "failed to assert that the same number of files were ignored") for _, file := range excludedFiles { baseName := filepath.Base(file) @@ -205,7 +249,7 @@ func TestGetGroupsWithOnlyLockFiles(t *testing.T) { setUp(true) path := "testdata/misc" const nbrOfGroups = 1 - fileGroups, err := finder.GetGroups(path, []string{"**/requirements*.txt"}, false, StrictAll) + fileGroups, err := finder.GetGroups(path, []string{"**/requirements*.txt", "**/composer.json", "**/composer.lock", "**/go.mod"}, false, StrictAll) assert.NoError(t, err) assert.Equalf(t, nbrOfGroups, fileGroups.Size(), "failed to assert that %d groups were created. %d was found", nbrOfGroups, fileGroups.Size()) @@ -221,21 +265,26 @@ func TestGetGroupsWithOnlyLockFiles(t *testing.T) { func TestGetGroupsWithTwoFileMatchesInSameDir(t *testing.T) { setUp(true) path := "testdata/pip" - const nbrOfGroups = 2 + const nbrOfGroups = 3 fileGroups, err := finder.GetGroups(path, []string{}, false, StrictAll) assert.NoError(t, err) assert.Equalf(t, nbrOfGroups, fileGroups.Size(), "failed to assert that %d groups were created. %d was found", nbrOfGroups, fileGroups.Size()) var files []string for _, fg := range fileGroups.groups { - assert.Len(t, fg.LockFiles, 1) - files = append(files, filepath.Base(fg.ManifestFile)) - relatedFile := fg.LockFiles[0] - if strings.Contains(fg.ManifestFile, "requirements.txt") { - assert.Contains(t, relatedFile, "requirements.txt.pip.debricked.lock") + if strings.Contains(fg.ManifestFile, "requirements.test.txt") { + assert.Len(t, fg.LockFiles, 0) } else { - assert.Contains(t, relatedFile, "requirements-dev.txt.pip.debricked.lock") + assert.Len(t, fg.LockFiles, 1) + relatedFile := fg.LockFiles[0] + if strings.Contains(fg.ManifestFile, "requirements.txt") { + assert.Contains(t, relatedFile, "requirements.txt.pip.debricked.lock") + } else { + assert.Contains(t, relatedFile, "requirements-dev.txt.pip.debricked.lock") + } } + files = append(files, filepath.Base(fg.ManifestFile)) + } assert.Contains(t, files, "requirements.txt") @@ -256,36 +305,36 @@ func TestGetGroupsWithStrictFlag(t *testing.T) { name: "StrictnessSetTo0", strictness: StrictAll, testedGroupIndex: 3, - expectedNumberOfGroups: 7, - expectedManifestFile: "requirements.txt", - expectedLockFiles: []string{}, + expectedNumberOfGroups: 10, + expectedManifestFile: "composer.json", + expectedLockFiles: []string{"composer.lock", "go.mod", "Cargo.lock", "requirements.txt.pip.debricked"}, }, { name: "StrictnessSetTo1", strictness: StrictLockAndPairs, testedGroupIndex: 1, - expectedNumberOfGroups: 5, + expectedNumberOfGroups: 6, expectedManifestFile: "", expectedLockFiles: []string{ - "Cargo.lock", + "composer.lock", "composer.lock", "go.mod", "Cargo.lock", "requirements.txt.pip.debricked", "requirements-dev.txt.pip.debricked", }, }, { name: "StrictnessSetTo2", strictness: StrictPairs, testedGroupIndex: 0, - expectedNumberOfGroups: 3, + expectedNumberOfGroups: 4, expectedManifestFile: "composer.json", expectedLockFiles: []string{ - "composer.lock", + "composer.lock", "requirements-dev.txt.pip.debricked.lock", "requirements.txt.pip.debricked.lock", }, }, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { - path := "testdata" - fileGroups, err := finder.GetGroups(path, []string{}, false, c.strictness) + filePath := "testdata" + fileGroups, err := finder.GetGroups(filePath, []string{}, false, c.strictness) fileGroup := fileGroups.groups[c.testedGroupIndex] assert.Nilf(t, err, "failed to assert that no error occurred. Error: %s", err) @@ -306,16 +355,23 @@ func TestGetGroupsWithStrictFlag(t *testing.T) { fileGroup.ManifestFile, c.expectedManifestFile, ) - - if len(c.expectedLockFiles) > 0 { - for i := range c.expectedLockFiles { + var expectedLockFiles []string + copy(expectedLockFiles, c.expectedLockFiles) + sort.Strings(expectedLockFiles) + lockFiles := make([]string, len(fileGroup.LockFiles)) + for i, filePath := range fileGroup.LockFiles { + lockFiles[i] = path.Base(filePath) + } + sort.Strings(lockFiles) + if len(expectedLockFiles) > 0 { + for i := range expectedLockFiles { assert.Containsf( t, - fileGroup.LockFiles[i], - c.expectedLockFiles[i], + lockFiles[i], + expectedLockFiles[i], "actual lock file %s doesn't match expected %s", - fileGroup.LockFiles[i], - c.expectedLockFiles[i], + lockFiles[i], + expectedLockFiles[i], ) } } diff --git a/internal/file/group.go b/internal/file/group.go index 310e7505..f1327f45 100644 --- a/internal/file/group.go +++ b/internal/file/group.go @@ -47,40 +47,42 @@ func (fileGroup *Group) GetAllFiles() []string { return append(files, fileGroup.LockFiles...) } -func (fileGroup *Group) checkFilePathDependantCases(fileMatch bool, lockFileMatch bool, file string) bool { - if lockFileMatch { - filePathDependantCases := fileGroup.getFilePathDependantCases() - for _, c := range filePathDependantCases { - if strings.HasSuffix(file, c) { - fileBase, _ := strings.CutSuffix(file, c) - - return len(fileGroup.ManifestFile) > 0 && (fileBase == filepath.Base(fileGroup.ManifestFile)) - } - } +func (fileGroup *Group) matchLockFile(lockFile, dir string) bool { + if !fileGroup.HasFile() { - return true + return false } + groupDir, manifestFile := filepath.Split(fileGroup.ManifestFile) + if groupDir != dir { - if fileMatch { - filePathDependantCases := fileGroup.getFilePathDependantCases() - for _, c := range filePathDependantCases { - for _, lockFile := range fileGroup.LockFiles { - if strings.HasSuffix(lockFile, c) { - lockFileBase, _ := strings.CutSuffix(lockFile, c) + return false + } - return lockFileBase == file - } - } - } + return matchFile(manifestFile, lockFile) +} + +func (fileGroup *Group) matchManifestFile(manifestFile, dir string) bool { + if fileGroup.HasFile() { - return true + return false } + groupDir, lockFile := filepath.Split(fileGroup.LockFiles[0]) + if groupDir != dir { - return true + return false + } + + return matchFile(manifestFile, lockFile) } -func (fileGroup *Group) getFilePathDependantCases() []string { - return []string{ - ".pip.debricked.lock", +func matchFile(manifestFile, lockFile string) bool { + var isPIP bool + lockFile, isPIP = strings.CutSuffix(lockFile, ".pip.debricked.lock") + if isPIP { + lockFile, _ = strings.CutPrefix(lockFile, ".") + + return lockFile == manifestFile } + + return true } diff --git a/internal/file/group_test.go b/internal/file/group_test.go index 555757d4..d00dabdd 100644 --- a/internal/file/group_test.go +++ b/internal/file/group_test.go @@ -47,47 +47,60 @@ func TestGetAllFiles(t *testing.T) { assert.Len(t, g.GetAllFiles(), 0, "failed to assert number of files") } -func TestCheckFilePathDependantCasesEmptyGroup(t *testing.T) { - group := NewGroup("", nil, []string{}) - var check bool - - check = group.checkFilePathDependantCases(true, false, "requirements.txt") - assert.True(t, check) - - check = group.checkFilePathDependantCases(true, false, "requirements-dev.txt") - assert.True(t, check) - - check = group.checkFilePathDependantCases(true, false, "requirements-dev.txt") - assert.True(t, check) - - check = group.checkFilePathDependantCases(false, true, "requirements.txt.pip.debricked.lock") - assert.False(t, check) +func TestGroupMatchLockFile(t *testing.T) { + var match bool + g1 := NewGroup("requirements.txt", nil, []string{}) + match = g1.matchLockFile("requirements.txt.pip.debricked.lock", "") + assert.Equal(t, match, true) + g2 := NewGroup("/home/requirements-test.txt", nil, []string{}) + match = g2.matchLockFile("requirements-test.txt.pip.debricked.lock", "/home/") + assert.Equal(t, match, true) + g3 := NewGroup("requirements.txt", nil, []string{}) + match = g3.matchLockFile("requirements.dev.txt.pip.debricked.lock", "") + assert.Equal(t, match, false) + g4 := NewGroup("requirements-test.txt", nil, []string{}) + match = g4.matchLockFile("requirements.txt.pip.debricked.lock", "") + assert.Equal(t, match, false) + g5 := NewGroup("requirements-test.txt", nil, []string{}) + match = g5.matchLockFile("requirements.txt-test.txt.pip.debricked.lock", "") + assert.Equal(t, match, false) + + // Check that match fails if different directories + g6 := NewGroup("requirements.txt", nil, []string{}) + match = g6.matchLockFile("requirements.txt.pip.debricked.lock", "some/other/directory") + assert.Equal(t, match, false) + + // Check that match fails if there is no manifest file - functionality may change in the future. + g7 := NewGroup("", nil, []string{}) + match = g7.matchLockFile("requirements.txt.pip.debricked.lock", "some/other/directory") + assert.Equal(t, match, false) } -func TestCheckFilePathDependantCasesWithFilePath(t *testing.T) { - group := NewGroup("requirements.txt", nil, []string{}) - var check bool - - check = group.checkFilePathDependantCases(false, true, "requirements.txt.pip.debricked.lock") - assert.True(t, check) - - check = group.checkFilePathDependantCases(false, true, "requirements-dev.txt.pip.debricked.lock") - assert.False(t, check) +func TestGroupMatchManifestFile(t *testing.T) { + var match bool + g1 := NewGroup("", nil, []string{"/home/requirements.txt.pip.debricked.lock"}) + match = g1.matchManifestFile("requirements.txt", "/home/") + assert.Equal(t, match, true) + + // Check that match fails if different directories + g6 := NewGroup("", nil, []string{"requirements.txt.pip.debricked.lock"}) + match = g6.matchManifestFile("requirements.txt", "some/other/directory") + assert.Equal(t, match, false) + + // Check that match fails when manifest file exists -- only one per group for now. + g7 := NewGroup("requirements.txt", nil, []string{}) + match = g7.matchManifestFile("requirements.dev.txt", "") + assert.Equal(t, match, false) } -func TestCheckFilePathDependantCasesWithLockFile(t *testing.T) { - group := NewGroup("", nil, []string{"requirements.txt.pip.debricked.lock"}) - var check bool - - check = group.checkFilePathDependantCases(true, false, "requirements.txt") - assert.True(t, check) - - check = group.checkFilePathDependantCases(true, false, "requirements-dev.txt") - assert.False(t, check) - - check = group.checkFilePathDependantCases(true, false, "requirements-dev.txt") - assert.False(t, check) - - check = group.checkFilePathDependantCases(false, false, "file.txt") - assert.True(t, check) +func TestGroupMatchFile(t *testing.T) { + var match bool + match = matchFile("package.json", "package-lock.json") + assert.Equal(t, match, true) + match = matchFile("requirements.txt", ".requirements.txt.pip.debricked.lock") + assert.Equal(t, match, true) + match = matchFile("requirements.txt", "requirements.txt.pip.debricked.lock") + assert.Equal(t, match, true) + match = matchFile("requirements-test.txt", ".requirements.txt.pip.debricked.lock") + assert.Equal(t, match, false) } diff --git a/internal/file/groups.go b/internal/file/groups.go index be33a912..328fa4be 100644 --- a/internal/file/groups.go +++ b/internal/file/groups.go @@ -19,21 +19,20 @@ func (gs *Groups) Match(format *CompiledFormat, path string, lockfileOnly bool) dir, file := filepath.Split(path) // If it is not a match, return - fileMatch := !lockfileOnly && format.MatchFile(file) + manifestFileMatch := !lockfileOnly && format.MatchFile(file) lockFileMatch := format.MatchLockFile(file) - if !fileMatch && !lockFileMatch { + if !manifestFileMatch && !lockFileMatch { return false } - matched := gs.matchExistingGroup(format, fileMatch, lockFileMatch, dir, file) - if matched { + if gs.groupExists(format, manifestFileMatch, lockFileMatch, dir, file) { return true } // Create new Group var newG *Group - if fileMatch { + if manifestFileMatch { newG = NewGroup(path, format, []string{}) } else { newG = NewGroup("", format, []string{path}) @@ -43,26 +42,20 @@ func (gs *Groups) Match(format *CompiledFormat, path string, lockfileOnly bool) return true } -func (gs *Groups) matchExistingGroup(format *CompiledFormat, fileMatch bool, lockFileMatch bool, dir string, file string) bool { +func (gs *Groups) groupExists(format *CompiledFormat, matchOnManifestFile bool, matchOnLockFile bool, dir string, file string) bool { for _, g := range gs.groups { - var groupDir string - if g.HasFile() { - groupDir, _ = filepath.Split(g.ManifestFile) - } else { - groupDir, _ = filepath.Split(g.LockFiles[0]) + if format != g.CompiledFormat { + continue } + if matchOnLockFile && g.matchLockFile(file, dir) { + g.LockFiles = append(g.LockFiles, dir+file) - matchesGroup := groupDir == dir && format == g.CompiledFormat && g.checkFilePathDependantCases(fileMatch, lockFileMatch, file) - if matchesGroup { - if fileMatch { - g.ManifestFile = dir + file + return true - return true - } else if lockFileMatch { - g.LockFiles = append(g.LockFiles, dir+file) + } else if matchOnManifestFile && g.matchManifestFile(file, dir) { + g.ManifestFile = dir + file - return true - } + return true } } diff --git a/internal/file/groups_test.go b/internal/file/groups_test.go index a9a1803e..e0c7eacf 100644 --- a/internal/file/groups_test.go +++ b/internal/file/groups_test.go @@ -148,3 +148,39 @@ func TestFilterGroupsByStrictness(t *testing.T) { }) } } + +func TestMatchGroupsExpected(t *testing.T) { + setUp(true) + + testData := map[string][]string{ + "foo/bar/cloud/package.json": {"foo/bar/cloud/yarn.lock"}, + "foo/bar/examples/test/requirements.txt": {}, + "foo/asd/requirements-test-dev.txt": {"foo/asd/.requirements-test-dev.txt.pip.debricked.lock"}, + "foo/asd/requirements-test.txt": {"foo/asd/.requirements-test.txt.pip.debricked.lock"}, + "foo/asd/requirements.txt": {"foo/asd/.requirements.txt.pip.debricked.lock"}, + "foo/asd/requirements-api.txt": {}, + "foo/asd/src/main/event_listeners/requirements.txt": {"foo/asd/src/main/event_listeners/.requirements.txt.pip.debricked.lock"}, + "foo/asd/src/main/util/test/composer.json": {}, + } + + var groups Groups + lockfileOnly := false + formats, _ := finder.GetSupportedFormats() + for key, values := range testData { + paths := append(values, key) + for _, path := range paths { + for _, format := range formats { + if groups.Match(format, path, lockfileOnly) { + + break + } + } + } + } + + for _, group := range groups.groups { + assert.Equal(t, testData[group.ManifestFile], group.LockFiles) + } + + assert.Equal(t, len(testData), len(groups.groups)) +} diff --git a/internal/file/testdata/pip/requirements.txt.pip.debricked.lock b/internal/file/testdata/misc/composer.json similarity index 100% rename from internal/file/testdata/pip/requirements.txt.pip.debricked.lock rename to internal/file/testdata/misc/composer.json diff --git a/internal/file/testdata/misc/composer.lock b/internal/file/testdata/misc/composer.lock new file mode 100644 index 00000000..e69de29b diff --git a/internal/file/testdata/misc/go.mod b/internal/file/testdata/misc/go.mod new file mode 100644 index 00000000..e69de29b diff --git a/internal/file/testdata/pip/.requirements.txt.pip.debricked.lock b/internal/file/testdata/pip/.requirements.txt.pip.debricked.lock new file mode 100644 index 00000000..e69de29b diff --git a/internal/file/testdata/pip/requirements.test.txt b/internal/file/testdata/pip/requirements.test.txt new file mode 100644 index 00000000..e69de29b