-
Notifications
You must be signed in to change notification settings - Fork 7
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
Use CTEST_PARALLEL_LEVEL #248
Conversation
builder/actions/cmake.py
Outdated
@@ -246,7 +246,7 @@ def run(self, env): | |||
return | |||
|
|||
ctest = toolchain.ctest_binary() | |||
sh.exec(*toolchain.shell_env, ctest, | |||
sh.exec(*toolchain.shell_env, ctest, "--parallel", str(os.cpu_count()), |
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.
Why? If this is affecting all repos then that's very concerning.
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.
the tests are slow. Hmmm, do we have tests that cannot run in parallel? If so, I guess I'll just change s3 to run in parallel instead
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.
I worry about crt-cpp where we have mqtt tests that used unique client ids but share topics and cross-talk between them
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.
Ideally all tests would be truly independent and mutually exclusive, but I don't think we're there yet.
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.
Sure, I'll just enable it only for aws-c-s3 then.
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.
Still needs to update builder for the script to import environment variables
move to just s3 |
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.