-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
test: add integration plugin tests #7299
test: add integration plugin tests #7299
Conversation
7fee04c
to
eeda1c8
Compare
integration/integration_test.go
Outdated
@@ -1,4 +1,4 @@ | |||
//go:build integration || vm_integration || module_integration || k8s_integration | |||
//go:build integration || vm_integration || module_integration || k8s_integration || plugin_integration |
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.
The VM, module, and K8s tests take longer to run and require a special setup. That's why they are separated. I think the plugin test can be part of the normal integration tests.
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.
updated in 33cf9d5
|
||
// Overwrite Stdout to get output of plugin | ||
defaultStdout := os.Stdout | ||
os.Stdout = f |
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.
What if passing io.Writer here?
trivy/integration/integration_test.go
Line 284 in 88ba460
app.SetOut(io.Discard) |
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 mean
func execute(osArgs []string, out io.Writer) error {
if out == nil {
out = io.Discard
}
...
}
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 tried to use your idea.
But we use os.Stdout
in plugin
package.
I used NewManager instead of defaultManager in app.go
to use Stdout
from cobra
.
It works, but there is problem with case when plugin is output.
We need to add output for plugin in Options:
Lines 511 to 533 in 0c6687d
func (o *Options) outputPluginWriter(ctx context.Context) (io.Writer, func() error, error) { | |
pluginName := strings.TrimPrefix(o.Output, "plugin=") | |
pr, pw := io.Pipe() | |
wait, err := plugin.Start(ctx, pluginName, plugin.Options{ | |
Args: o.OutputPluginArgs, | |
Stdin: pr, | |
}) | |
if err != nil { | |
return nil, nil, xerrors.Errorf("plugin start: %w", err) | |
} | |
cleanup := func() error { | |
if err = pw.Close(); err != nil { | |
return xerrors.Errorf("failed to close pipe: %w", err) | |
} | |
if err = wait(); err != nil { | |
return xerrors.Errorf("plugin error: %w", err) | |
} | |
return nil | |
} | |
return pw, cleanup, nil | |
} |
i am not sure that we need to do that now.
I would leave this workaround until the tests become more numerous and complex, which would require these changes.
But if you want to do it - tell me and I will continue the research
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.
OK, we can improve it later.
|
||
// Overwrite Stdout to get output of plugin | ||
defaultStdout := os.Stdout | ||
os.Stdout = f |
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.
OK, we can improve it later.
Description
See #7276
CI/CD test - https://github.com/aquasecurity/trivy/actions/runs/10214386342/job/28261673506?pr=7299#step:5:1044
Related issues
Checklist