-
Notifications
You must be signed in to change notification settings - Fork 126
Add support for runtime fields and test packages #2409
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
Conversation
761d9a2
to
5aa613a
Compare
test integrations |
Created or updated PR in integrations repository to test this version. Check elastic/integrations#12790 |
Co-authored-by: Jaime Soriano Pastor <jaime.soriano@elastic.co>
💚 Build Succeeded
History
cc @mrodm |
// Known issue: do not include this as part of the xUnit results | ||
// Example: https://buildkite.com/elastic/integrations/builds/22313#01950431-67a5-4544-a720-6047f5de481b/706-2459 | ||
var pathErr *fs.PathError | ||
if errors.As(err, &pathErr) && pathErr.Op == "fork/exec" && pathErr.Path == "/usr/bin/docker" { | ||
return result.WithError(err) | ||
} | ||
// report all other errors as error entries in the xUnit file | ||
results, _ := result.WithError(err) | ||
return results, nil | ||
} |
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.
This is based on
elastic-package/internal/testrunner/runners/static/tester.go
Lines 118 to 122 in 3aaadce
hasBenchmark, err := stream.StaticValidation(ctx, stream.NewOptions(withOpts...), r.testFolder.DataStream) | |
if err != nil { | |
results, _ := resultComposer.WithError(err) | |
return results | |
} |
WDYT @jsoriano ?
This would be required to allow to catch the errors of the false positive test package introduced in this PR. I guess it could be updated the script to get the latest line something like from the stderr output (not tested).
As advantage, this would help to report unhealthy containers as errors (related to #2039).
The disadvantage of this is that any error from prepareScenario
would be reported as an error in xUnit. Just added here the exception for the docker error that it is a known issue. It happens from time to time in CI builds.
Should we do the same with the response of validateTestScenario
? For this one, I think I would keep it as it is for now.
https://github.com/elastic/elastic-package/pull/2409/files#diff-0bdd696fb9abd5e4aa12a3bab634cbac31ec1a1349b3a61e8a784a6bdeb6e7d8R1766
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.
It LGTM like this.
As we have the CI now, everything that goes to the xunit files sounds good to me, as this makes easier to find what failed in jobs.
Relates #1229
Closes #2208
Closes #2039
This PR adds support for runtime fields.
runtime
setting.Additional:
prepareScenario
(system tests) are now reported as errors in the xUnit file.