-
Notifications
You must be signed in to change notification settings - Fork 638
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
refactor(neard/tests): reduce boilerplate with intuitive structures #4229
refactor(neard/tests): reduce boilerplate with intuitive structures #4229
Conversation
• Serially execute every child that's actively running. • Kill the parent of every child that's not yet running so they panic and die of shock. Everybody Dies™ :-)
a delay on start_nodes while a SIGINT is issued\ncould
this way, the main thread (always) awaits completion of its kids the behaviour is a lot more concise, reasonable and effective
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.
Overall looks goods!
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.
@miraclx Great job! I have a few ideas in the comments
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.
👍 on turning setup code into setup data!
4a57aae
to
56fba0d
Compare
save the extra import
not all tasks within async context are non-blocking issuing a SIGINT while on a blocking task was getting ignored stop actix with exit code 130 in the unix way 🤓
49fbc59
to
8c9ed4a
Compare
I am not sure why we were catching ctrl+c in the first place. This was introduced in near#4229 I think. But messing up with signal handlers in tests doesn't feel right and, indeed, prevents tests from being killed. Test Plan --------- Manual testing: running `cargo t -p integration-tests` and then killing it in the middle with ctrl+c leaves the test running in the background before this PR, and properly kills the process after.
I am not sure why we were catching ctrl+c in the first place. This was introduced in near#4229 I think. But messing up with signal handlers in tests doesn't feel right and, indeed, prevents tests from being killed. Test Plan --------- Manual testing: running `cargo t -p integration-tests` and then killing it in the middle with ctrl+c leaves the test running in the background before this PR, and properly kills the process after.
I am not sure why we were catching ctrl+c in the first place. This was introduced in #4229 I think. But messing up with signal handlers in tests doesn't feel right and, indeed, prevents tests from being killed. Test Plan --------- Manual testing: running `cargo t -p integration-tests` and then killing it in the middle with ctrl+c leaves the test running in the background before this PR, and properly kills the process after.
I am not sure why we were catching ctrl+c in the first place. This was introduced in #4229 I think. But messing up with signal handlers in tests doesn't feel right and, indeed, prevents tests from being killed. Test Plan --------- Manual testing: running `cargo t -p integration-tests` and then killing it in the middle with ctrl+c leaves the test running in the background before this PR, and properly kills the process after.
neard/tests/*
are simplified with easy to reason about structures.