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

Segregate integration tests into different processes. #295

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

LuisPH3
Copy link
Contributor

@LuisPH3 LuisPH3 commented Nov 11, 2024

This PR segregates the different integration tests into their own package names. Therefore, they will be compiled into separate binaries which go test will execute in parallel.
This is done to speedup execution of unit tests, which during race detection can easily exceed timeout settings.

In addition:

  • Count and test-net-test reuse running integration networks to avoid overhead
  • Timeout time for the integration network startup has been increased to 5 minutes. Because race-enabled tests would time-out during initialization of the network, and produce false positives.

This pr is part of https://github.com/Fantom-foundation/sonic-admin/issues/50
This pr helps towards https://github.com/Fantom-foundation/sonic-admin/issues/47.

@LuisPH3 LuisPH3 requested a review from facuMH November 11, 2024 12:12
@LuisPH3 LuisPH3 marked this pull request as draft November 12, 2024 09:42
@LuisPH3 LuisPH3 force-pushed the luis/segregate branch 2 times, most recently from cf34a96 to d98ed18 Compare November 12, 2024 10:30
tests/integration_test_net.go Outdated Show resolved Hide resolved
tests/counter/counter_test.go Outdated Show resolved Hide resolved
@LuisPH3 LuisPH3 force-pushed the luis/segregate branch 2 times, most recently from b470c75 to 37f4233 Compare November 12, 2024 11:13
HerbertJordan
HerbertJordan previously approved these changes Nov 13, 2024
Copy link
Collaborator

@HerbertJordan HerbertJordan left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks for improving the life of us developers!

You might get into memory usage issues by running all tests in parallel. If you do, you can use the -p option for limiting the number of parallel processes go test is using to run tests.

@LuisPH3
Copy link
Contributor Author

LuisPH3 commented Nov 13, 2024

This PR has been moved back to draft because of a recurring failure in CI:

When running, go test ./tests/... everything seems to be alright. But when running go test ./... there are spurious test failures and/or termination signal received. This makes me think that there is yet some shared resource that does not get randomized correctly.

Because the race detector did not find any new race, when running it locally. I will park this work for the time being and focus on other tasks.
I will come back to this issue once the other issues are dispatched.

@LuisPH3 LuisPH3 force-pushed the luis/segregate branch 4 times, most recently from 9f39f44 to 3aa5bf0 Compare November 20, 2024 14:38
@LuisPH3 LuisPH3 self-assigned this Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants