-
Notifications
You must be signed in to change notification settings - Fork 426
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
feat(test): e2e integration test for new helpers #4354
feat(test): e2e integration test for new helpers #4354
Conversation
@rscampos Maybe use these helpers in the existing signatures if possible? For example add themto the instrumentation tests. Then we can merge. |
b85e0fb
to
bedbced
Compare
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.
LGTM, although we should probably use something similar to the flow events in DNS instead of these events (which are meant for legacy compatibility iirc).
go.mod
Outdated
@@ -43,6 +43,8 @@ require ( | |||
sigs.k8s.io/controller-runtime v0.18.2 | |||
) | |||
|
|||
replace github.com/aquasecurity/tracee/signatures/helpers => ./signatures/helpers/ |
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.
Remove and update through types PR
@rscampos Merge at will once you do the types PR (which you can merge without +1 as far as i'm concerned) |
bedbced
to
5a60f6f
Compare
5a60f6f
to
f457c2b
Compare
/fast-forward |
This comment was marked as outdated.
This comment was marked as outdated.
/fast-forward |
This comment was marked as duplicate.
This comment was marked as duplicate.
/fast-forward |
Triggered from #4354 (comment) by @geyslan. Trying to fast forward Target branch ( commit 8f63209c1dc5ecd7e08fce2ebaf2eb3f802068a3 (HEAD -> main, origin/main, origin/HEAD)
Author: Gregório G. <geyslan@gmail.com>
Date: Tue Dec 3 17:25:38 2024 -0300
fix(tests): possible out of range in integration (#4305)
* fix(tests): possible out of range in integration
ExpectAllInOrderSequentially might try to access an index out of range
depending on the number of events that are being checked.
More about in issue #4255.
* fix(tests): use ExpectAtLeastOneForEach
Two test cases were using ExpectAllInOrderSequentially helper function
and passing by luck, since they emit more events than expected only
beyond the expected events boundary.
For that cases we should use ExpectAtLeastOneForEach helper function
instead. Pull request ( commit f457c2bf6cffc49a0ed9c04d3d35a842f6802fe7 (pull_request/4346_net_packet_http_request)
Author: Raphael Campos <raphaelcampos.rp@gmail.com>
Date: Fri Oct 18 08:35:53 2024 -0500
feat(test): e2e integration test for new helpers Fast forwarding $ git push origin f457c2bf6cffc49a0ed9c04d3d35a842f6802fe7:main
To https://github.com/aquasecurity/tracee.git
! [remote rejected] f457c2bf6cffc49a0ed9c04d3d35a842f6802fe7 -> main (refusing to allow a GitHub App to create or update workflow `.github/workflows/pr.yaml` without `workflows` permission)
error: failed to push some refs to 'https://github.com/aquasecurity/tracee.git' |
Or, you're touching workflows. That's why. In this case you need to use the conventional Github button. |
fix: #4346
1. Explain what the PR does
f457c2b feat(test): e2e integration test for new helpers
2. Explain how to test it
net_packet_http_request
ornet_packet_http_response
.GetProtoHTTPRequestByName
orGetProtoHTTPResponseByName
.3. Other comments