Skip to content
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

fix: make gator output relative paths #2443

Merged

Conversation

acpana
Copy link
Contributor

@acpana acpana commented Dec 9, 2022

Signed-off-by: Alex Pana 8968914+acpana@users.noreply.github.com

fixes #1640

This patch attaches the InputPath / OriginalPath to a Suite. We still use the absolute path for file lookups at run time (runTest).

Examples of how this works now in practice:

# relative dirs expand as expected
$ gator verify ./test/gator/verify/...

ok      ./test/gator/verify/more_suites/even_more/suite copy 2.yaml     0.007s
ok      ./test/gator/verify/more_suites/even_more/suite copy 3.yaml     0.010s
ok      ./test/gator/verify/more_suites/even_more/suite copy.yaml       0.007s
ok      ./test/gator/verify/more_suites/even_more/suite.yaml    0.009s
ok      ./test/gator/verify/more_suites/suite copy 2.yaml       0.006s
ok      ./test/gator/verify/more_suites/suite copy 3.yaml       0.009s
ok      ./test/gator/verify/more_suites/suite copy.yaml 0.008s
ok      ./test/gator/verify/more_suites/suite.yaml      0.010s
ok      ./test/gator/verify/suite.yaml  0.007s
PASS

# notice change for output without `./`
$ gator verify test/gator/verify/... 

ok      test/gator/verify/more_suites/even_more/suite copy 2.yaml       0.013s
ok      test/gator/verify/more_suites/even_more/suite copy 3.yaml       0.007s
ok      test/gator/verify/more_suites/even_more/suite copy.yaml 0.008s
ok      test/gator/verify/more_suites/even_more/suite.yaml      0.009s
ok      test/gator/verify/more_suites/suite copy 2.yaml 0.007s
ok      test/gator/verify/more_suites/suite copy 3.yaml 0.009s
ok      test/gator/verify/more_suites/suite copy.yaml   0.007s
ok      test/gator/verify/more_suites/suite.yaml        0.010s
ok      test/gator/verify/suite.yaml    0.007s
PASS

# yaml file
$ gator verify test/gator/verify/suite.yaml

ok      test/gator/verify/suite.yaml    0.010s
PASS

# pass absolute path as input, get absolute path
$ gator verify /pwd/gatekeeper/test/gator/verify/suite.yaml

ok      /pwd/gatekeeper/test/gator/verify/suite.yaml   0.007s
PASS

Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
@codecov-commenter
Copy link

codecov-commenter commented Dec 9, 2022

Codecov Report

Base: 53.63% // Head: 53.60% // Decreases project coverage by -0.02% ⚠️

Coverage data is based on head (e3243c1) compared to base (febba65).
Patch coverage: 85.71% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2443      +/-   ##
==========================================
- Coverage   53.63%   53.60%   -0.03%     
==========================================
  Files         117      117              
  Lines       10281    10293      +12     
==========================================
+ Hits         5514     5518       +4     
- Misses       4346     4352       +6     
- Partials      421      423       +2     
Flag Coverage Δ
unittests 53.60% <85.71%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/gator/result.go 72.72% <ø> (ø)
pkg/gator/read_suites.go 86.25% <83.33%> (-1.14%) ⬇️
pkg/gator/runner.go 85.54% <100.00%> (ø)
pkg/watch/replay.go 78.97% <0.00%> (-2.28%) ⬇️
...onstrainttemplate/constrainttemplate_controller.go 56.45% <0.00%> (-0.96%) ⬇️
pkg/readiness/object_tracker.go 83.98% <0.00%> (+1.06%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@@ -89,10 +89,11 @@ func runE(cmd *cobra.Command, args []string) error {
if strings.HasSuffix(originalPath, "/...") {
recursive = true
targetPath = strings.TrimSuffix(targetPath, "...")
originalPath = strings.TrimSuffix(originalPath, "...")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in this file, right above lines 68-79 see how we create a fileSystem .

Since we pass that in and the absolute path, it's hard to print the results with the input path, given that we can traverse a directory.

Another way to tackle this problem, where we don't pass in the InputPath would be to push down the fileSystem/ absolute path creation to the point where we walk the directory or where we try to read a file. But I think that approach would require a more sweeping refactor.

Copy link
Contributor

@maxsmythe maxsmythe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for doing this!

@maxsmythe maxsmythe requested review from ritazh and sozercan December 9, 2022 21:23
Copy link
Member

@ritazh ritazh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
Thanks for the PR! @acpana!

@maxsmythe maxsmythe merged commit ea4c297 into open-policy-agent:master Dec 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[gator test] Make suite paths relative to working directory in results output
4 participants