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

RMN integration test updates #15423

Open
wants to merge 13 commits into
base: develop
Choose a base branch
from
Open

RMN integration test updates #15423

wants to merge 13 commits into from

Conversation

dimkouv
Copy link
Contributor

@dimkouv dimkouv commented Nov 26, 2024

  • Re-configure and enable the disabled test cases in CI.
  • Add cursing related integration tests (no issues found).
  • Refactor the test, split testCase runner body into functions.

Copy link
Contributor

github-actions bot commented Nov 26, 2024

AER Report: CI Core ran successfully ✅

aer_workflow , commit

AER Report: Operator UI CI ran successfully ✅

aer_workflow , commit

@dimkouv dimkouv changed the title enable rmn tests RMN integration test updates Nov 28, 2024
@@ -75,7 +82,7 @@ func TestRMN_MultipleMessagesOnOneLaneNoWaitForExec(t *testing.T) {
func TestRMN_NotEnoughObservers(t *testing.T) {
runRmnTestCase(t, rmnTestCase{
name: "one message but not enough observers, should not get a commit report",
passIfNoCommitAfter: time.Minute, // wait for a minute and assert that commit report was not delivered
passIfNoCommitAfter: 15 * time.Second,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, aren't we too aggressive with these durations? What is the commit is not delivered in 15 seconds, because something was lagging behind or the node didn't reach consensus, not because of the real reason?

I think that's a general problem with tests waiting for something "to not happen" within a time range. Is there any more reliable way of asserting that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's one of my concerns as-well, I believe we do the same in other integration tests.
I couldn't figure out some better way. Any suggestions?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a general issue with our testing framework that needs to be addressed at a higher level, we have tons of tests like this in smoke that are waiting to flake given ambiguous guestimates on timing. I agree with @mateusz-sekara and more generally I think smoke tests should test the integration of components/systems without executing under a "wait and see" model.

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.

3 participants