Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: Add tests for core engine functionality #477

Merged
merged 16 commits into from Feb 5, 2021
Merged

feat: Add tests for core engine functionality #477

merged 16 commits into from Feb 5, 2021

Conversation

simar7
Copy link
Member

@simar7 simar7 commented Jan 28, 2021

This PR adds tests for some of the core engine functionality.

Signed-off-by: Simarpreet Singh simar@linux.com

@simar7
Copy link
Member Author

simar7 commented Jan 28, 2021

Looks like the CI runner only runs the tests in tracee-ebpf dir. Any ideas how to run tests in all packages (tracee-rules, etc.)?

@itaysk
Copy link
Collaborator

itaysk commented Jan 28, 2021

#479 please review

@simar7 simar7 marked this pull request as ready for review January 28, 2021 20:30
@simar7 simar7 changed the title feat: Add tests for core engine functionality feat: Add tests for consumeEngine functionality Jan 28, 2021
@simar7 simar7 changed the title feat: Add tests for consumeEngine functionality feat: Add tests for core engine functionality Jan 28, 2021
simar7
simar7 commented Jan 28, 2021
View reviewed changes
go.mod Outdated Show resolved Hide resolved
simar7
simar7 commented Jan 28, 2021
View reviewed changes
tracee-rules/signature.go Outdated Show resolved Hide resolved
@simar7 simar7 self-assigned this Jan 29, 2021
itaysk
itaysk reviewed Jan 29, 2021
View reviewed changes
Copy link
Collaborator

@itaysk itaysk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @simar7 ! I've added a few comments but also, I think the last commit (dc437b3) doesn't belong in this PR as it's a changing to the business logic, not just the tests. I also think that commit is unnecessary, but we can discuss this separately from the tests PR.

tracee-rules/engine/engine.go Outdated Show resolved Hide resolved
tracee-rules/engine/engine.go Outdated Show resolved Hide resolved
tracee-rules/engine/engine.go Outdated Show resolved Hide resolved
tracee-rules/engine/engine_test.go Outdated Show resolved Hide resolved
tracee-rules/engine/engine_test.go Outdated Show resolved Hide resolved
tracee-rules/signature.go Outdated Show resolved Hide resolved
itaysk
itaysk reviewed Jan 29, 2021
View reviewed changes
Copy link
Collaborator

@itaysk itaysk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idk this was omitted from the original review but it's important comment

tracee-rules/engine/engine.go Outdated Show resolved Hide resolved
@simar7 simar7 mentioned this pull request Feb 2, 2021
Closed
@simar7 simar7 requested a review from itaysk February 2, 2021 21:03
yanivagman
yanivagman approved these changes Feb 4, 2021
View reviewed changes
Copy link
Collaborator

@yanivagman yanivagman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!
GJ @simar7

tracee-rules/engine/engine_test.go Outdated Show resolved Hide resolved
tracee-rules/engine/engine_test.go Outdated Show resolved Hide resolved
@simar7
engine_test: Add a simple test for consumeSource
c04ec81 
Signed-off-by: Simarpreet Singh <simar@linux.com>
@simar7
engine_test: Add additional test cases for consumeSources
ebe900d 
Signed-off-by: Simarpreet Singh <simar@linux.com>
@simar7
feat: engine_test.go: Add more testcases for for consumeSources
8a615ac 
Signed-off-by: Simarpreet Singh <simar@linux.com>
@simar7
feat: engine_test.go: Add a few sad paths for consumeSources
406a40c 
Signed-off-by: Simarpreet Singh <simar@linux.com>
@simar7
feat: engine_test.go: Add more cases and fix a faulty logger
5e44efd 
Signed-off-by: Simarpreet Singh <simar@linux.com>
@simar7
fix: Remove accidentaly commited go.mod
1966139 
Signed-off-by: Simarpreet Singh <simar@linux.com>
@simar7
fix: signature.go: Un-export GetSignatures func.
1c37050 
Signed-off-by: Simarpreet Singh <simar@linux.com>
@simar7
fix: engine.go: Don't defer in a loop. Don't nil set input channel.
50477f6 
Setting the engine.inputs.Tracee = nil does nothing and is not required.

defer shouldn't be called within a loop, we should rather do it in the
responsible goroutine.

Signed-off-by: Simarpreet Singh <simar@linux.com>
@simar7
fix: go mod tidy
4329ee6 
Signed-off-by: Simarpreet Singh <simar@linux.com>
@simar7
Revert "fix: engine.go: Don't defer in a loop. Don't nil set input ch…
0ac0a7b 
…annel."

This reverts commit dc437b3.
@simar7
fix: engine_test.go: Remove some cruft from test cases
627d02c 
Signed-off-by: Simarpreet Singh <simar@linux.com>
@simar7
fix: engine.go: Drop error if failure happens within sig
cb02a86 
Signed-off-by: Simarpreet Singh <simar@linux.com>
@simar7
fix: engine.go Remove cruft comments
0dfc80b 
Signed-off-by: Simarpreet Singh <simar@linux.com>
@simar7
fix: Cleanup test name and cases.
2cd3c45 
Signed-off-by: Simarpreet Singh <simar@linux.com>
@simar7
fix: Rebase/ resolve conflicts
2105f3a 
Signed-off-by: Simarpreet Singh <simar@linux.com>
@simar7
fix: engine_test.go: Loosen the assertion on logs
2dfb7ca 
Signed-off-by: Simarpreet Singh <simar@linux.com>
@simar7
Copy link
Member Author

simar7 commented Feb 4, 2021

I had to rebase on top of latest main so could you just double check if all is well @itaysk before I press the big green button to merge?

itaysk
itaysk approved these changes Feb 5, 2021
View reviewed changes
Copy link
Collaborator

@itaysk itaysk left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @simar7 LGTM
please squash before merging

@simar7 simar7 merged commit 3590ef0 into aquasecurity:main Feb 5, 2021
@simar7 simar7 deleted the improvements-2 branch February 5, 2021 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@itaysk itaysk itaysk approved these changes

@yanivagman yanivagman yanivagman approved these changes

Assignees

@simar7 simar7

Labels
area/rules
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@simar7 @itaysk @yanivagman @grantseltzer