Update 'compute lint' to use linter plugins. #1125
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1125 +/- ##
==========================================
+ Coverage 69.95% 70.30% +0.35%
==========================================
Files 145 146 +1
Lines 12007 12068 +61
==========================================
+ Hits 8399 8485 +86
+ Misses 2879 2839 -40
- Partials 729 744 +15
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
…eed to be public, but they don't).
| connection.SetConfig(config) | ||
|
|
||
| applyCmd := apply.Command() | ||
| applyCmd.SetArgs([]string{"-f", "../complexity/testdata/apigeeregistry", "-R"}) |
There was a problem hiding this comment.
Rather than copy this directory here, I am just picking it out of its nearby location, expecting to follow up with a PR that just moves this test data around (with no other changes, to make it easier to review).
| if err != nil { | ||
| return fmt.Errorf("failed to get linter from flags: %s", err) | ||
| } | ||
| if linter == "" { |
There was a problem hiding this comment.
There was a problem hiding this comment.
Thanks! I will fix this before merging.
| } | ||
| args[0] = c.FQName(args[0]) | ||
|
|
||
| linter, err := cmd.Flags().GetString("linter") | ||
| if err != nil { | ||
| return fmt.Errorf("failed to get linter from flags: %s", err) |
There was a problem hiding this comment.
All of these Flags().GetXXX() error checks could be removed by using variable-bound flags.
There was a problem hiding this comment.
Agreed, and that also improves our test coverage :) I'll create an issue for a general review and update of our flag construction.
| err := visitor.FetchSpecContents(ctx, task.client, spec) | ||
| root, err := conformance.WriteSpecForLinting(ctx, task.client, task.spec) | ||
| if root != "" { | ||
| defer os.RemoveAll(root) |
There was a problem hiding this comment.
Is it ok for our commands to blindly wipe out without an overwrite flag?
There was a problem hiding this comment.
This is removing a temporary directory, so I think it's ok here, but I could imagine a --debug flag that preserves them. Maybe a follow-up issue?
This partly addresses #1121 by updating
registry compute lintto use the linter plugins that were created forregistry compute conformance. To simplify testing, aregistry-lint-testplugin is added that returns sizes of all files-to-be-linted.registry compute lintnow also proactively verifies that a specified linter plugin is available (i.e.--linter testrequires an available executable namedregistry-lint-test).Much of the code needed to run linters was factored out of functions specific to
registry compute conformanceand is now shared by thecompute lintandcompute conformancecommands.