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

Fix flaky entity store persistence test #2540

Conversation

albinsuresh
Copy link
Contributor

@albinsuresh albinsuresh commented Dec 20, 2023

Proposed changes

The tests were failing due to a time precision problem while validating MQTT messages with a date_from range and not because there was a duplicate message. The date_from value takes a UNIX timestamp without milli-second precision. But in the failing test case, the following was happening (note the time precision):

  1. Entity registration message published at 10:10:35.513002
  2. Converted message received by the logger at 10:10:36.198326
  3. The timestamp to be used in the date_from for the assertion was generated using Get Unix Timestamp API at 10:10:36.756000. But, the Get Unix Timestamp returns the time: 1703067036 without millisecond precision.

When that timestamp without precision, which is 10:10:36.000000 is used as the date_from in the assertion, it covers the original message as well, and that wrongly detects it as a duplicate message.

To make sure that there is a delay of more than 1 second between the point at which the converted message was emitted and the point of assertion, the managed object existence checks were added in between which should add a significant delay.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Improvement (general improvements like code refactoring that doesn't explicitly fix a bug or add any new functionality)
  • Documentation Update (if none of the other choices apply)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Paste Link to the issue


Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA (in all commits with git commit -s)
  • I ran cargo fmt as mentioned in CODING_GUIDELINES
  • I used cargo clippy as mentioned in CODING_GUIDELINES
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

Copy link
Contributor

github-actions bot commented Dec 20, 2023

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass % ⏱️ Duration
378 0 3 378 100 1h1m50.675999999s

@albinsuresh albinsuresh force-pushed the debug-entity-store-persistence-test-failure branch from 50fe38a to 88b6b61 Compare December 20, 2023 11:08
@albinsuresh albinsuresh force-pushed the debug-entity-store-persistence-test-failure branch from 88b6b61 to ba4e56d Compare December 20, 2023 13:24
@albinsuresh albinsuresh changed the title Debug test Fix flaky entity store persistence test Dec 20, 2023
Comment on lines +168 to +174
External Identity Should Exist plc1
External Identity Should Exist plc2
External Identity Should Exist plc1-sensor1
External Identity Should Exist plc1-sensor2
External Identity Should Exist plc2-sensor1
External Identity Should Exist plc1-metrics
External Identity Should Exist plc2-metrics
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even a simple 1s sleep here would have solved the issue. But, doing a deterministic assertion here looked better.

@albinsuresh albinsuresh marked this pull request as ready for review December 20, 2023 13:46
@albinsuresh albinsuresh merged commit 9c78246 into thin-edge:main Dec 20, 2023
16 checks passed
@albinsuresh albinsuresh deleted the debug-entity-store-persistence-test-failure branch January 19, 2024 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants