Skip to content

Conversation

@EugenioCollado
Copy link
Contributor

This PR improves the robustness of test_reliability_no_losses to resolve intermittent failures observed in test_reliability_4.

Changes include:

  • Added a retry mechanism to explicitly wait for samples if they are not immediately found in the buffer, fixing timing-related flakiness.

  • Added safer checks for None to prevent potential execution errors when parsing process output.

Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>
@angelrti
Copy link
Contributor

angelrti commented Jan 8, 2026

probably we should replace this function with another one that relies on an increasing sample size (-z 0). I've developed that in a different branch: https://github.com/omg-dds/dds-rtps/blob/add_new_test_python_0725/test_suite_functions.py#L356
Used: https://github.com/omg-dds/dds-rtps/blob/add_new_test_python_0725/test_suite.py#L190
It also works with only 1 instance.

It is simpler and less error-prone


# If the sample is not found in the buffer, wait for it
if sub_string is None:
index = child_sub.expect(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this shouldn't be needed because whenever we call this function, we have already matched a sample (from the interoperability_report.py)

@EugenioCollado
Copy link
Contributor Author

EugenioCollado commented Jan 12, 2026

probably we should replace this function with another one that relies on an increasing sample size (-z 0). I've developed that in a different branch: https://github.com/omg-dds/dds-rtps/blob/add_new_test_python_0725/test_suite_functions.py#L356 Used: https://github.com/omg-dds/dds-rtps/blob/add_new_test_python_0725/test_suite.py#L190 It also works with only 1 instance.

It is simpler and less error-prone

Agreed, this looks like a better solution. I’ll close this PR in favor of yours. Please make sure to use the new check_function in test_reliability_4, and consider removing the other one as well.

@angelrti
Copy link
Contributor

Yes, I will change that

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.

2 participants