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

Fixes for federated tests #1794

Merged
merged 7 commits into from
May 30, 2023
Merged

Conversation

petervdonovan
Copy link
Collaborator

This is an effort to partially correct some of the Python federated test failures that we are observing.

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.
@petervdonovan petervdonovan changed the title Fix federated tests2 Fixes for federated tests May 28, 2023
@petervdonovan petervdonovan force-pushed the fix-federated-tests2 branch from 79c428f to 4c05b68 Compare May 29, 2023 18:09
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.
@petervdonovan
Copy link
Collaborator Author

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.
@petervdonovan
Copy link
Collaborator Author

The Python federated tests all pass for me locally. CI is failing due to the expired Klighd certificate.

@petervdonovan petervdonovan marked this pull request as ready for review May 29, 2023 23:44
Copy link
Member

@lhstrh lhstrh left a 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!

Copy link
Collaborator

@byeonggiljun byeonggiljun left a 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).

@byeonggiljun byeonggiljun merged commit 2157755 into fix-ts-federated-tests May 30, 2023
@cmnrd cmnrd deleted the fix-federated-tests2 branch June 8, 2023 07:44
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.

None yet

3 participants