Skip to content

Commit c6c045b

Browse files
authored
👻 Adding Fixes for the broken tests (#1106)
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved Windows drive-path handling so exclusions and tracked files behave correctly on Windows. * **Refactor** * More accurate path matching and relative-path computation, plus deduplication of aggregated search results for more reliable filtering. * **Tests** * Expanded test coverage for regex-based exclusions and tightened test setup with explicit init/teardown and increased verbosity. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Shawn Hurley <shawn@hurley.page>
1 parent 56d0a37 commit c6c045b

File tree

5 files changed

+76
-20
lines changed

5 files changed

+76
-20
lines changed

provider/internal/builtin/service_client.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -500,7 +500,12 @@ func (b *builtinServiceClient) getWorkingCopies() ([]string, []string) {
500500
excludedPaths := []string{}
501501
for _, wc := range b.workingCopyMgr.getWorkingCopies() {
502502
additionalIncludedPaths = append(additionalIncludedPaths, wc.wcPath)
503-
excludedPaths = append(excludedPaths, wc.filePath)
503+
if runtime.GOOS == "windows" && wc.drive != "" {
504+
path := filepath.Join(wc.drive, wc.filePath[1:])
505+
excludedPaths = append(excludedPaths, path)
506+
} else {
507+
excludedPaths = append(excludedPaths, wc.filePath)
508+
}
504509
}
505510
return additionalIncludedPaths, excludedPaths
506511
}

provider/internal/builtin/service_client_test.go

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -681,11 +681,6 @@ func Test_builtinServiceClient_Evaluate_InclusionExclusion(t *testing.T) {
681681
},
682682
},
683683
},
684-
chainTemplate: engine.ChainTemplate{
685-
Filepaths: []string{
686-
filepath.Join("dir_b", "dir_a", "ba.xml"),
687-
},
688-
},
689684
notifiedFileChanges: []provider.FileChange{
690685
{
691686
Path: filepath.Join(baseLocation, "dir_b", "b.xml"),
@@ -776,12 +771,14 @@ func Test_builtinServiceClient_Evaluate_InclusionExclusion(t *testing.T) {
776771
config: provider.InitConfig{
777772
Location: baseLocation,
778773
},
779-
log: testr.New(t),
774+
log: testr.NewWithOptions(t, testr.Options{
775+
Verbosity: 10,
776+
}),
780777
includedPaths: tt.includedPathsFromConfig,
781778
excludedDirs: tt.excludedPathsFromConfig,
782779
locationCache: map[string]float64{},
783780
cacheMutex: sync.RWMutex{},
784-
workingCopyMgr: NewTempFileWorkingCopyManger(testr.New(t)),
781+
workingCopyMgr: NewTempFileWorkingCopyManger(testr.NewWithOptions(t, testr.Options{Verbosity: 10})),
785782
}
786783
p.workingCopyMgr.init()
787784
defer p.workingCopyMgr.stop()
@@ -987,6 +984,8 @@ func Test_builtinServiceClient_Evaluate_ExcludeDirs(t *testing.T) {
987984
cacheMutex: sync.RWMutex{},
988985
workingCopyMgr: NewTempFileWorkingCopyManger(testr.New(t)),
989986
}
987+
sc.workingCopyMgr.init()
988+
defer sc.workingCopyMgr.stop()
990989
conditionInfo, err := yaml.Marshal(&tt.condition)
991990
if err != nil {
992991
t.Errorf("builtinServiceClient.Evaluate() invalid test case, please check if condition is correct")

provider/internal/builtin/wcmanger.go

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@ package builtin
22

33
import (
44
"context"
5+
"fmt"
56
"os"
67
"path/filepath"
7-
"regexp"
88
"runtime"
99
"strings"
1010
"sync"
@@ -17,6 +17,7 @@ import (
1717
type workingCopy struct {
1818
filePath string
1919
wcPath string
20+
drive string
2021
}
2122

2223
type workingCopyManager struct {
@@ -77,10 +78,19 @@ func (t *workingCopyManager) reformatIncidents(incidents ...provider.IncidentCon
7778
inc := &incidents[i]
7879
if strings.HasPrefix(string(inc.FileURI), "file://") &&
7980
strings.HasPrefix(inc.FileURI.Filename(), t.tempDir) {
80-
inc.FileURI = uri.File(
81-
filepath.Clean(strings.Replace(
82-
inc.FileURI.Filename(), t.tempDir, "", -1)))
81+
if runtime.GOOS == "windows" {
82+
parts := strings.Split(strings.ReplaceAll(inc.FileURI.Filename(), t.tempDir, ""), string(filepath.Separator))
83+
t.log.Info("parts here", "parts", parts, "temp-dir", t.tempDir)
84+
driveLetter := fmt.Sprintf("%s:\\", parts[1])
85+
newParts := []string{driveLetter}
86+
newParts = append(newParts, parts[2:]...)
87+
inc.FileURI = uri.File(filepath.Clean(filepath.Join(newParts...)))
88+
} else {
89+
inc.FileURI = uri.File(filepath.Clean(strings.ReplaceAll(
90+
inc.FileURI.Filename(), t.tempDir, "")))
91+
}
8392
}
93+
8494
formatted = append(formatted, *inc)
8595
}
8696
return formatted
@@ -96,9 +106,11 @@ func (t *workingCopyManager) startWorker() {
96106
return
97107
}
98108
// we need to get rid of volume label on windows
109+
var drive string
99110
if runtime.GOOS == "windows" {
100-
volLabel := regexp.MustCompile(`^[a-zA-Z]:`)
101-
change.Path = volLabel.ReplaceAllString(change.Path, "")
111+
drive = filepath.VolumeName(change.Path)
112+
driveUpdated, _ := strings.CutSuffix(drive, ":")
113+
change.Path = strings.ReplaceAll(change.Path, drive, driveUpdated)
102114
}
103115
_, wcExists := t.workingCopies[change.Path]
104116
wcPath := filepath.Join(t.tempDir, change.Path)
@@ -131,6 +143,7 @@ func (t *workingCopyManager) startWorker() {
131143
t.workingCopies[change.Path] = workingCopy{
132144
filePath: change.Path,
133145
wcPath: wcPath,
146+
drive: drive,
134147
}
135148
t.wcMutex.Unlock()
136149
}

provider/lib.go

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"path/filepath"
1212
"regexp"
1313
"runtime"
14+
"slices"
1415
"strings"
1516

1617
"github.com/go-logr/logr"
@@ -181,6 +182,7 @@ func (f *FileSearcher) Search(s SearchCriteria) ([]string, error) {
181182
finalSearchResult = append(finalSearchResult, files...)
182183
}
183184

185+
finalSearchResult = dedupSlice(finalSearchResult...)
184186
// apply baseline include patterns and any search patterns
185187
finalSearchResult = f.filterFilesByPathsOrPatterns(statFunc, includedPatterns, finalSearchResult, false)
186188
// apply patterns from search criteria
@@ -227,19 +229,32 @@ func (f *FileSearcher) filterFilesByPathsOrPatterns(statFunc cachedOsStat, patte
227229
} else {
228230
rPattern := pattern
229231
// if the pattern doesn't contain a wildcard, do an exact match only
230-
if regexp.QuoteMeta(pattern) == pattern {
232+
if !strings.ContainsAny(pattern, "*?[]$^") {
231233
rPattern = "^" + pattern + "$"
232234
}
233235
// try matching as go regex pattern
234-
relPath, err := filepath.Rel(f.BasePath, file)
235-
if err != nil {
236-
// This should never happen, if paths are found outside the base path, then something did not occur
237-
// correctly.
236+
var relPath string
237+
if strings.HasPrefix(file, f.BasePath) {
238+
// This is not in a working copy manager or some other additional path
239+
// We want to just search for matches within the base path.
240+
var err error
241+
relPath, err = filepath.Rel(f.BasePath, file)
242+
if err != nil {
243+
f.Log.Error(fmt.Errorf("unable to get relative path for file from base path"),
244+
"this should not happen, please file a bug", "basePath", f.BasePath, "filePath", file)
245+
continue
246+
}
247+
relPath = filepath.Join(string(os.PathSeparator), relPath)
248+
} else if slices.Contains(f.AdditionalPaths, file) {
249+
// When this comes from an addional path, we won't know
250+
// where to search from, so fall back to the full file path.
251+
relPath = file
252+
} else {
238253
f.Log.Error(fmt.Errorf("unable to get relative path for file from base path"),
239254
"this should not happen, please file a bug", "basePath", f.BasePath, "filePath", file)
240255
continue
241256
}
242-
relPath = filepath.Join(string(os.PathSeparator), relPath)
257+
243258
f.Log.V(9).Info("using regex to search", "pattern", pattern, "relPath", relPath)
244259
regex, regexErr := regexp.Compile(rPattern)
245260
if regexErr == nil && (regex.MatchString(relPath) || regex.MatchString(filepath.Base(file))) {

provider/lib_test.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -432,6 +432,30 @@ func TestFileSearcher_Search(t *testing.T) {
432432
},
433433
wantErr: false,
434434
},
435+
{
436+
name: "exclude pattern with regex",
437+
basePath: testBasePath,
438+
providerConfigConstraints: IncludeExcludeConstraints{
439+
IncludePathsOrPatterns: []string{},
440+
ExcludePathsOrPatterns: []string{`.*\.txt$`},
441+
},
442+
searchCriteria: SearchCriteria{
443+
Patterns: []string{},
444+
ConditionFilepaths: []string{},
445+
},
446+
wantFilePatterns: []string{
447+
".mvn/maven-wrapper.jar",
448+
"node_modules/package.json",
449+
"src/helper.go",
450+
"lib/util.go",
451+
"lib/common.go",
452+
"vendor/dep.go",
453+
"README.md",
454+
"config.yaml",
455+
"src/main.go",
456+
},
457+
wantErr: false,
458+
},
435459
{
436460
name: "include pattern with wildcard",
437461
basePath: testBasePath,

0 commit comments

Comments
 (0)