-
Notifications
You must be signed in to change notification settings - Fork 51
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
add wolfictl test command #743
Conversation
bf06f8d
to
d31b42c
Compare
as is, this always writes logs to their |
I think I'd prefer just forking |
+1 I had been thinking it would be pretty much like this but just called from wolfictl: And then we should pretty much just replace this: with wolfictl test |
1171ee3
to
41abe46
Compare
ack, its forked, ptal! I took some liberty with how logs should be dumped (stdout for 1 package, file for n packges), lmk if it sucks |
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.
Thanks! Just a little nit, but that's just my preference🤣
cmd.Flags().StringSliceVar(&cfg.extraPackages, "test-package-append", []string{"wolfi-base"}, "extra packages to install for each of the test environments") | ||
cmd.Flags().StringVar(&cfg.cacheDir, "cache-dir", "./melange-cache/", "directory used for cached inputs") | ||
cmd.Flags().StringVar(&cfg.cacheSource, "cache-source", "", "directory or bucket used for preloading the cache") | ||
cmd.Flags().StringVar(&cfg.dst, "destination-repository", "", "repo where packages will eventually be uploaded, used to skip existing packages (currently only supports http)") |
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.
Nit: I don't think we need this, since we're not building anything
return fmt.Errorf("getting packages: %w", err) | ||
} | ||
|
||
todoPkgs := make(map[string]struct{}, len(packages)) |
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.
nit: I would find it easier to read this, if we constructed the list of packages that we will test here up front instead of creating these two data structures (pkgs, todoPkgs) and then in the for loop below filter things out. Something like, if len(packages) > 0, then maybe use pkgs.Sub(packages...) otherwise just use the full one. Thinking being that I'd expect the common case to be 1 or few packages.
https://github.com/wolfi-dev/wolfictl/blob/v0.15.18/pkg/dag/packages.go#L349
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.
good call out, this was a relic of the earlier approach attempting to reuse build
, I'll refactor this in a follow up
since this is essentially all new independent code, I'm going to merge this as is and address the nits in a follow up so I don't have to bug yall for reacks here 🙏 there's also chainguard-dev/melange#1134 I'll include in a follow up |
adds
wolfictl test
.attempts to reuse things that make sense from
wolfictl build
, but since we don't really care about executing the tests in the appropriate dag order, the amount of re-use is pretty minimal.this uses a simpler
errgroup
to dispatch (in alphabetical order).this also supports testing a single, multiple, or all packages depending on the positional args provided.
sample use case for this: wolfi-dev/os#16351