-
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 "exclude" in package config #709
Support "exclude" in package config #709
Conversation
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.
I have one change but otherwise this is good to go. Please address that and we can get this merged.
pkg/config/config.go
Outdated
p := pathlib.NewPath(subPkg) | ||
relativePath, err := p.RelativeToStr(pkgPath) | ||
if err != nil { | ||
return fmt.Errorf("failed to get path for %s relative to %s: %w", subPkg, pkgPath, err) |
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.
Please change this to use the StackErr: https://github.com/vektra/mockery/blob/master/pkg/stackerr/stackerr.go#L21
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.
Done in 1407ff8
exclude: | ||
- subpkg3 | ||
`, | ||
wantCfgMap: `dir: foobar |
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.
I understand about the funkiness of the test. That's okay, people can come back to this PR if they're confused :)
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #709 +/- ##
===================================================
- Coverage 74.89121% 74.85912% -0.03209%
===================================================
Files 9 9
Lines 2298 2307 +9
===================================================
+ Hits 1721 1727 +6
- Misses 440 442 +2
- Partials 137 138 +1
☔ View full report in Codecov by Sentry. |
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.
Looks good, will merge when tests pass.
Description
This PR recognizes the "exclude" directive in a package configuration. The following configuration snippet now excludes the "client/common" directory (and everything therein):
This is motivated by errors we get from mockery arising from imports in
github.com/organization/reponame/core/clients/common
. We want to be able to use the recursive functionality (since we have a bunch of different clients) but skip the one directory that has only test helpers and no interfaces to mock. The "exclude" feature lets us do that in the old style configuration format, but there was no implementation of this in the new packages format.Fixes #708
Type of change
Version of Golang used when building/testing:
How Has This Been Tested?
Added a unit test to the existing table driven test that exercises the revised functionality. (It may look a bit strange to add a stanza where the input YAML equals the output YAML. However without the change in the code, the output YAML would be auto-populated with the information about
subpkg3
).Tested in our use case, as running
mockery
against our configuration (represented by the YAML snippet above) correctly logs that it is skipping the subpackage and exits without error.Checklist
Regarding documentation updates:
exclude
is in the table of deprecated parameters for v3. I am hoping that this use case allows it to be brought back.