-
Notifications
You must be signed in to change notification settings - Fork 63
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
Fixes for federated tests #1794
Conversation
This might fix the issue where one failing test (that times out) causes all the remaining tests to fail. Timeouts occur when a federate dies because apparently the RTI doesn't quit when one federate dies (maybe it should), but that's a separate issue.
79c428f
to
4c05b68
Compare
For some reason, creating a trap for SIGTERM results in SIGTERM not causing the program to exit. In any case, we really do not want the program to exit without killing the RTI and federates under any circumstance unless we are sure that the program terminated successfully. Therefore, it seems safest to require positive confirmation via an EXITED_SUCCESSFULLY flag that the program really did exit successfully.
This replaces all instances in the test/Python/src directory.
I think that the zombie RTI problem is probably fixed now. I didn't even have to do anything terribly hacky. Looking at the logs that we are now getting in CI, it seems possible that many (most/all???) of the errors that we are getting could be trivial to fix. |
This is not the right solution. The right solution is not to do the files property merging in the first place. This has been discusssed. I think everyone agrees about this, but probably it will not be fixed until the package manager is more complete.
The Python federated tests all pass for me locally. CI is failing due to the expired Klighd certificate. |
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.
This generally looks good to me! If the tests pass locally, then let's go ahead and merge this into @byeong-gil's branch. Thanks a bunch for helping out with this, @petervdonovan!
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.
LGTM! Thank you for helping to address these issues, @petervdonovan! I'll merge this to my PR (lingua-franca#1752).
This is an effort to partially correct some of the Python federated test failures that we are observing.