-
Notifications
You must be signed in to change notification settings - Fork 7.1k
Add multi-node integration test for symmetric_run. #59875
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
base: master
Are you sure you want to change the base?
Conversation
e984cb3 to
c09619a
Compare
Signed-off-by: Ricardo Decal <[email protected]> Signed-off-by: Ricardo Decal <[email protected]>
Signed-off-by: Ricardo Decal <[email protected]> Signed-off-by: Ricardo Decal <[email protected]>
Signed-off-by: Ricardo Decal <[email protected]> Signed-off-by: Ricardo Decal <[email protected]>
Signed-off-by: Ricardo Decal <[email protected]> Signed-off-by: Ricardo Decal <[email protected]>
Signed-off-by: Ricardo Decal <[email protected]> Signed-off-by: Ricardo Decal <[email protected]>
Signed-off-by: Ricardo Decal <[email protected]> Signed-off-by: Ricardo Decal <[email protected]>
Signed-off-by: Ricardo Decal <[email protected]> Signed-off-by: Ricardo Decal <[email protected]>
Signed-off-by: Ricardo Decal <[email protected]> Signed-off-by: Ricardo Decal <[email protected]>
Signed-off-by: Ricardo Decal <[email protected]> Signed-off-by: Ricardo Decal <[email protected]>
Signed-off-by: Ricardo Decal <[email protected]> Signed-off-by: Ricardo Decal <[email protected]>
Signed-off-by: Ricardo Decal <[email protected]> Signed-off-by: Ricardo Decal <[email protected]>
Signed-off-by: Ricardo Decal <[email protected]> Signed-off-by: Ricardo Decal <[email protected]>
Signed-off-by: Ricardo Decal <[email protected]> Signed-off-by: Ricardo Decal <[email protected]>
Signed-off-by: Ricardo Decal <[email protected]> Signed-off-by: Ricardo Decal <[email protected]>
Signed-off-by: Ricardo Decal <[email protected]>
Signed-off-by: Ricardo Decal <[email protected]>
c09619a to
2143067
Compare
|
ready for review @richardliaw |
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.
Code Review
This pull request is a solid improvement, adding a valuable multi-node integration test for symmetric_run.py and refactoring existing code for better robustness and clarity. The use of a wrapper script to simulate a multi-node environment is clever, and the argument handling improvements in symmetric_run.py are well-implemented. I have a few suggestions to further enhance the robustness and readability of the new test code.
Signed-off-by: Ricardo Decal <[email protected]>
Signed-off-by: Ricardo Decal <[email protected]>
Signed-off-by: Ricardo Decal <[email protected]>
Signed-off-by: Ricardo Decal <[email protected]>
Description
symmetric_run.py:test_symmetric_run_three_node_cluster_simulated. I did this by creating a wrapper script in order to mock network interfaces within subprocesses.--head=true