Skip to content

Added one more message to the test - #5552

Merged
Aaronontheweb merged 20 commits into
akkadotnet:devfrom
brah-mcdude:FishForMessage_that_returns_all_the_messages
Feb 11, 2022
Merged

Added one more message to the test#5552
Aaronontheweb merged 20 commits into
akkadotnet:devfrom
brah-mcdude:FishForMessage_that_returns_all_the_messages

Conversation

@brah-mcdude

@brah-mcdude brah-mcdude commented Jan 30, 2022 •
edited by Aaronontheweb
Loading

Copy link
Copy Markdown
Contributor

close #5551

@brah-mcdude

brah-mcdude commented Jan 30, 2022

Copy link
Copy Markdown
Contributor Author

The issue for this PR:
FishForMessage should allow an allMessages optional parameter that can return all the messages until isMessage returns true #5551

@brah-mcdude brah-mcdude mentioned this pull request Jan 30, 2022
Closed
Aaronontheweb
Aaronontheweb requested changes Feb 1, 2022
View reviewed changes

@Aaronontheweb Aaronontheweb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Need to support our API compatibility guidelines - see https://getakka.net/community/contributing/api-changes-compatibility.html#how-to-safely-introduce-public-api-changes-extend-only-design for an explanation on how to introduce these types of changes in a binary-compatible way.

Comment thread src/core/Akka.TestKit/TestKitBase_Receive.cs Outdated
@Aaronontheweb Aaronontheweb added the akka-testkit Akka.NET Testkit issues label Feb 2, 2022
Aaronontheweb
Aaronontheweb reviewed Feb 4, 2022
View reviewed changes
Comment thread src/core/Akka.TestKit/TestKitBase_Receive.cs Outdated
Aaronontheweb
Aaronontheweb approved these changes Feb 4, 2022
View reviewed changes

@Aaronontheweb Aaronontheweb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@Aaronontheweb
Aaronontheweb enabled auto-merge (squash) February 4, 2022 17:11
auto-merge was automatically disabled February 4, 2022 19:40

Head branch was pushed to by a user without write access

Aaronontheweb
Aaronontheweb requested changes Feb 9, 2022
View reviewed changes

@Aaronontheweb Aaronontheweb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

API suggestion before merge

Comment thread src/core/Akka.TestKit/TestKitBase_Receive.cs
Arkatufus
Arkatufus approved these changes Feb 10, 2022
View reviewed changes

@Arkatufus Arkatufus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@Arkatufus
Arkatufus enabled auto-merge (squash) February 10, 2022 15:15
@Aaronontheweb

Aaronontheweb commented Feb 10, 2022

Copy link
Copy Markdown
Member

Have some conflicts on this one

@Aaronontheweb

Aaronontheweb commented Feb 10, 2022

Copy link
Copy Markdown
Member

Looks we have several test failures here too

@Arkatufus
Arkatufus disabled auto-merge February 10, 2022 20:33
Aaronontheweb
Aaronontheweb requested changes Feb 10, 2022
View reviewed changes

@Aaronontheweb Aaronontheweb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Akka.Testkit.Tests.TestKitBaseTests.ReceiveTests.WaitForRadioSilenceAsync_should_reset_timer_twice_only
Expected messages to be a collection with 2 item(s), but found an empty collection.

@brah-mcdude brah-mcdude mentioned this pull request Feb 10, 2022
Closed
@brah-mcdude

brah-mcdude commented Feb 10, 2022

Copy link
Copy Markdown
Contributor Author

Akka.Testkit.Tests.TestKitBaseTests.ReceiveTests.WaitForRadioSilenceAsync_should_reset_timer_twice_only Expected messages to be a collection with 2 item(s), but found an empty collection.

I created a separate issue:
test WaitForRadioSilenceAsync_should_reset_timer_twice_only fails with empty collection #5640

And I submitted a PR to hopefully fix this issue:
slow down the test - WaitForRadioSilenceAsync_should_reset_timer_twice_only #5641

@Aaronontheweb

Aaronontheweb commented Feb 11, 2022

Copy link
Copy Markdown
Member

thanks @brah-mcdude - going to see if your other PR helped

@brah-mcdude

brah-mcdude commented Feb 11, 2022

Copy link
Copy Markdown
Contributor Author

@Aaronontheweb - please let me know if everything is ok or if I need to modify / fix anything.

@Aaronontheweb
Aaronontheweb merged commit cf49198 into akkadotnet:dev Feb 11, 2022
@brah-mcdude
brah-mcdude deleted the FishForMessage_that_returns_all_the_messages branch February 11, 2022 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Aaronontheweb Aaronontheweb Aaronontheweb requested changes
+1 more reviewer
@Arkatufus Arkatufus Arkatufus approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@Arkatufus Arkatufus

Labels

akka-testkit Akka.NET Testkit issues api-change

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

FishForMessage should allow an allMessages optional parameter that can return all the messages until isMessage returns true

3 participants

@brah-mcdude @Aaronontheweb @Arkatufus