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

fix(tests): fix goroutines leakage in integration tests #4047

Merged
merged 1 commit into from
May 16, 2024
Merged

fix(tests): fix goroutines leakage in integration tests #4047

merged 1 commit into from
May 16, 2024

Conversation

AlonZivony
Copy link
Collaborator

1. Explain what the PR does

Fix tests so they will make sure to close channels properly and clean goroutines.

Fix #4044

2. Explain how to test it

3. Other comments

@AlonZivony
fix(tests): fix goroutines leakage in integration tests
17c1589 
Fix tests so they will make sure to close channels properly and clean goroutines.
NDStrahilevitz
NDStrahilevitz approved these changes May 15, 2024
View reviewed changes
Copy link
Collaborator

@NDStrahilevitz NDStrahilevitz left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM, nice job, let's wait for tests to run and see this resolves.

@AlonZivony
Copy link
Collaborator Author

I will soon push some small change so don't merge yet

@AlonZivony AlonZivony force-pushed the bugfix/integration-unexpected-goroutines branch from f40e35b to 7fa46a7 Compare May 15, 2024 15:25
@AlonZivony
Copy link
Collaborator Author

I think we will also need to backport it to 0.21.0 so it won't cause problems there.
Do you agree with me?

@NDStrahilevitz
Copy link
Collaborator

I think we will also need to backport it to 0.21.0 so it won't cause problems there. Do you agree with me?

Not unlikely we will need more backports there in the near future, so I agree.

@AlonZivony AlonZivony force-pushed the bugfix/integration-unexpected-goroutines branch from 7fa46a7 to 17c1589 Compare May 15, 2024 15:50
@AlonZivony
Copy link
Collaborator Author

@NDStrahilevitz can I merge the PR?
I ask because I pushed a minor change and I am not sure if you went over it

yanivagman
yanivagman approved these changes May 16, 2024
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.

LGTM

@yanivagman yanivagman merged commit cd7ed0d into aquasecurity:main May 16, 2024
32 checks passed
AlonZivony added a commit to AlonZivony/tracee that referenced this pull request May 16, 2024
@AlonZivony
fix(tests): fix goroutines leakage in integration tests (aquasecurity…
12f41be 
…#4047)

Fix tests so they will make sure to close channels properly and clean goroutines.

(cherry picked from commit cd7ed0d)
@AlonZivony AlonZivony mentioned this pull request May 16, 2024
Merged
yanivagman pushed a commit that referenced this pull request May 16, 2024
@AlonZivony
fix(tests): fix goroutines leakage in integration tests (#4047) (#4052)
1ba8a3d 
Fix tests so they will make sure to close channels properly and clean goroutines.

(cherry picked from commit cd7ed0d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yanivagman yanivagman yanivagman approved these changes

@NDStrahilevitz NDStrahilevitz NDStrahilevitz approved these changes

Assignees

@AlonZivony AlonZivony

Labels
area/testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Flaky integration tests because of unexpected goroutines
3 participants
@AlonZivony @NDStrahilevitz @yanivagman