-
Notifications
You must be signed in to change notification settings - Fork 413
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support directory exclusion when generating mocks #458
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -68,9 +68,11 @@ func (w *Walker) doWalk(ctx context.Context, p *Parser, dir string, visitor Walk | |
|
||
files, err := ioutil.ReadDir(dir) | ||
if err != nil { | ||
log.Err(err).Msgf("Error reading directory") | ||
return | ||
} | ||
|
||
OUTER: | ||
for _, file := range files { | ||
if strings.HasPrefix(file.Name(), ".") || strings.HasPrefix(file.Name(), "_") { | ||
continue | ||
|
@@ -80,6 +82,17 @@ func (w *Walker) doWalk(ctx context.Context, p *Parser, dir string, visitor Walk | |
|
||
if file.IsDir() { | ||
if w.Recursive { | ||
nextFiles, err := ioutil.ReadDir(path) | ||
if err != nil { | ||
log.Err(err).Msgf("Error reading directory") | ||
return | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because the doWalk function doesn't provide a way to bubble the error up, I feel like this should be a fatal error. But, we really should fix |
||
} | ||
for _, nextFile := range nextFiles { | ||
if nextFile.Name() == ".mockery_skip" { | ||
continue OUTER | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tend to really avoid labels if at all possible. Actually, I think we can Shameless plug but you might want to look into https://pkg.go.dev/github.com/chigopher/pathlib and then you can just do |
||
} | ||
} | ||
|
||
generated = w.doWalk(ctx, p, path, visitor) || generated | ||
if generated && w.LimitOne { | ||
return | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -120,3 +120,30 @@ func TestPackagePrefix(t *testing.T) { | |
w.Walk(context.Background(), gv) | ||
assert.Regexp(t, regexp.MustCompile("package prefix_test_test"), bufferedProvider.String()) | ||
} | ||
|
||
func TestWalkerDirectorySkip(t *testing.T) { | ||
if testing.Short() { | ||
t.Skip("skipping recursive walker test") | ||
} | ||
|
||
wd, err := os.Getwd() | ||
assert.NoError(t, err) | ||
w := Walker{ | ||
BaseDir: wd, | ||
Recursive: true, | ||
LimitOne: false, | ||
Filter: regexp.MustCompile(".*"), | ||
} | ||
|
||
gv := NewGatheringVisitor() | ||
|
||
// Generating mocks, ignoring the fixtures directory | ||
skipMarker, err := os.Create(getFixturePath(".mockery_skip")) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's use pathlib for this. |
||
assert.NoError(t, err) | ||
defer func() { os.Remove(skipMarker.Name()) }() | ||
|
||
w.Walk(context.Background(), gv) | ||
|
||
// There are 4 mocks generated from the files within pkg/* but outside pkg/fixtures/* | ||
assert.Len(t, gv.Interfaces, 4) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, let's call this
.mockery_skip_all
. I envision we might want a.mockery_skip
that skips just a single directory, whereas the_all
can skip the entire tree.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By "skip the entire tree" vs "just a single directory" is the intended meaning that
? (checking I've got that right in my head)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep that sounds right, I feel like we might want both options at some point but we can just focus on "skip the entire tree" as you were originally doing. Thanks!