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.

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

Re-enable TestGroup_Go and fix flaky behavior #41230

Merged
merged 1 commit into from
Oct 15, 2024

Conversation

mauri870
Copy link
Member

@mauri870 mauri870 commented Oct 14, 2024

Proposed commit message

This PR re-enables the TestGroup_Go test, which was skipped in #41223. It also includes fixes to mitigate flakiness in the test logger caused by timming issues and ensures proper waiting for success conditions within the tests.

How to test this PR locally

$ cd filebeat/input/filestream/internal/task 
$ go test -c
$ go run golang.org/x/tools/cmd/stress@latest ./task.test -test.run TestGroup_Go -test.v

Related issues

@mauri870 mauri870 added flaky-test Unstable or unreliable test cases. Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team backport-8.x Automated backport to the 8.x branch with mergify labels Oct 14, 2024
@mauri870 mauri870 self-assigned this Oct 14, 2024
@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Oct 14, 2024
@mauri870 mauri870 marked this pull request as ready for review October 14, 2024 17:24
@mauri870 mauri870 requested a review from a team as a code owner October 14, 2024 17:24
@elasticmachine
Copy link
Collaborator

Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane)

@belimawr
Copy link
Contributor

@mauri870 did you run this test with the race detector enabled?

Looking at the improvements you did, it looks like the older version of the test should have been flagged by it 🤔

@mauri870
Copy link
Member Author

mauri870 commented Oct 15, 2024

Looking at the improvements you did, it looks like the older version of the test should have been flagged by it 🤔

You are right, perhaps it was not a race condition per-se. I noticed that sometimes the contents of the log would not include log messages from both goroutines, even when both had finished executing already. I think the changes to the logger are still welcome tho, it is now safe to use in multiple goroutines without syncronization.

@mauri870 mauri870 merged commit 66dacd9 into elastic:main Oct 15, 2024
30 checks passed
mergify bot pushed a commit that referenced this pull request Oct 15, 2024
mauri870 added a commit that referenced this pull request Oct 15, 2024
(cherry picked from commit 66dacd9)

Co-authored-by: Mauri de Souza Meneguzzo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-8.x Automated backport to the 8.x branch with mergify flaky-test Unstable or unreliable test cases. Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team
Projects
None yet
4 participants