fix(orc8r): Stabilize cloud tests

[MoritzThomasHuebner]

Closes #13998

Signed-off-by: Moritz Huebner moritz.huebner@tngtech.com

Summary

Upon inspecting the recent cloud-workflow failures, we found the following flaky tests:

• TestSqlConfiguratorStorage_LoadEntities: 2 failures
• TestGetSubscriberState: 2 failures
• TestSingletonRunSuccess: 6 failures
• TestSingletonRunFail: 4 failures
• TestSyncSubscribers: 1 failure
• TestRunBrokenIndexer: 1 failures

This PR focuses on TestSingletonRunSuccess, TestSingletonRunFail, and TestRunBrokenIndexer.

These changes focus on test isolation:

• Added manual calls to indexer.DeregisterAllForTest(t) at the end of the tests. This is not strictly necessary to make the tests green, but it can cause issues with dangling indexers if the order of the tests is ever reversed.

TestSingletonRunFail:

• Moved the creation of the reindexer thread after the registration of the indexers. This way, it is impossible that the reindexer is triggered when only one or two out of the three indexers are registered.

TestSingletonRunSuccess:

• The issue is that the TestHookReindexDone is called even if there is an error during reindexing. Unexpected errors can be triggered in the remote indexer getIndexerClient.
• Added a TestHookReindexFailure, that we use to keep track of reindexing job errors.
• Safely pull all elements out of the channel ch using recvChSafe. This function keeps track of failures and will pull the requisite number of elements from the channel.

TestRunBrokenIndexer:

• The issue is that in rare cases the reindexer can run twice before the indexer is registered.
• Moved the creation of the reindexer thread after the registration of the indexer, similar to what we do in TestSingletonRunFail.
• Removed one recvCh call as this appears to be unnecessary now,

Test Plan

TestSingletonRunSuccess:

• Reproduced error locally by manually adding an error that would trigger a fixed number of times in the remote indexer getIndexerClient.
• The fix in this PR fixes the error that occurs when we manually insert an error.
• It was not possible to recreate this issue on the CI, since copy/pasting the test did not appear to increase the likelihood compared to a single run.

TestSingletonRunFail

• Reproduced error by re-running many times on the CI initially.
• After adding the change, the test is green even with many re-runs: workflow.

TestRunBrokenIndexer:

• Error output can be reproduced locally by deleting one of the recvCh calls in the function.
• After adding the change, the test is green locally.
• Unfortunately, the test can not be re-run on the github runner many times in a single workflow as it fails for a different reason when running the second time.

Additional Information

• This change is backwards-breaking

[github-actions]

Thanks for opening a PR! 💯

A couple initial guidelines

• All commits must be signed off. This is enforced by DCO check.
• All PR titles must follow the semantic commits format. This is enforced by Semantic PR.

Howto

• Reviews. The "Reviewers" listed for this PR are the Magma maintainers who will shepherd it.
• Checks. All required CI checks must pass before merge.
• Merge. Once approved and passing CI checks, use the ready2merge label to indicate the maintainers can merge your PR.

More info

Please take a moment to read through the Magma project's

• Contributing Conventions for norms around contributed code

If this is your first Magma PR, also consider reading

• Developer Onboarding for onboarding as a new Magma developer
• Development Workflow for guidance on your first pull request
• CI Checks for points of contact for failing or flaky CI checks
• GitHub-to-Slack mappings for Magma maintainers for guidance on how to contact maintainers on Slack

[github-actions]

cloud-workflow

1 135 tests   1 135 ✔️  2m 25s ⏱️
   365 suites         0 💤
       7 files           0 ❌

Results for commit 55750d9.

♻️ This comment has been updated with latest results.

[github-actions]

feg-workflow

    2 files  203 suites   40s ⏱️
374 tests 374 ✔️ 0 💤 0 ❌
388 runs  388 ✔️ 0 💤 0 ❌

Results for commit 55750d9.

♻️ This comment has been updated with latest results.

[github-actions]

dp-workflow

14 tests   14 ✔️  2m 16s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit 55750d9.

♻️ This comment has been updated with latest results.

[github-actions]

agw-workflow

615 tests   611 ✔️  3m 48s ⏱️
    2 suites      4 💤
    2 files        0 ❌

Results for commit 55750d9.

♻️ This comment has been updated with latest results.

[sebathomas]

This didn't exactly make the test easier to understand. ;) I think it's looking good but there's a few changes I'm not sure about. Maybe let's go over it tomorrow in person.