Skip to content

Comments

fix: racy test in AskSpec#32111

Merged
patriknw merged 2 commits intoakka:mainfrom
leviramsey:ask-spec-fix
Sep 20, 2023
Merged

fix: racy test in AskSpec#32111
patriknw merged 2 commits intoakka:mainfrom
leviramsey:ask-spec-fix

Conversation

@leviramsey
Copy link
Contributor

@leviramsey leviramsey commented Sep 20, 2023

The particular message in the TimeoutException when an actor being asked terminates depends on whether the termination happens before the ask pattern checks for termination.

Per @Roiocam, the intent of the racy test is to exercise a scenario also set up (in a non-racy way) by another test, so this change now consolidates those cases.

result shouldBe a[TimeoutException]
result.getMessage should startWith("Ask timed out on")
// Message in the TimeoutException depends on the race between the actor stopping and
// the PromiseRef initializing... it's not really important for this test
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could also check that exactly one of the patterns (^Ask timed out on.*$ or ^Recipient.* had already terminated.$) matches, if we want to test that this only times out in one of two ways, but not sure on the value there.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, the original purpose of this line is mock an actor dead, and expected both future failure and dead letters.
Therefore, the "ask timeout out" is an mistake on PR #32104.

case DeadLetter(Foo(s, _), _, recipient) =>
s should ===("bar")

val classicRef = ActorRefAdapter.toClassic(ref)
Copy link
Contributor

Choose a reason for hiding this comment

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

Great consolidate, thanks.

Copy link
Contributor

@patriknw patriknw left a comment

Choose a reason for hiding this comment

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

LGTM

@patriknw patriknw merged commit 132058f into akka:main Sep 20, 2023
@patriknw patriknw added this to the 2.9.0-M2 milestone Sep 20, 2023
@leviramsey leviramsey deleted the ask-spec-fix branch September 20, 2023 14:52
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