Skip to content

Communication - Refactor Chat Tests to use StepVerifier#16601

Merged
minnieliu merged 4 commits intoAzure:masterfrom
minnieliu:refactorChatTests
Oct 21, 2020
Merged

Communication - Refactor Chat Tests to use StepVerifier#16601
minnieliu merged 4 commits intoAzure:masterfrom
minnieliu:refactorChatTests

Conversation

@minnieliu
Copy link
Member

No description provided.

@minnieliu
Copy link
Member Author

/azp run java - communication - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jbeauregardb
Copy link
Contributor

LGTM 👍🏻

// Arrange
CreateChatThreadOptions threadRequest = ChatOptionsProvider.createThreadOptions(
firstThreadMember.getId(), secondThreadMember.getId());
ChatThreadAsyncClient chatThreadClient = client.createChatThread(threadRequest).block();
Copy link
Member

Choose a reason for hiding this comment

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

still need to remove this block. It's what I was asking Srikanta about during the tutorial. Let me find his answer about how to do it

Copy link
Member

Choose a reason for hiding this comment

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

"What if we need to set up/create an object on the service first (as part of the arrange), before acting on it? Do we initialize the sync client too, and use it to arrange the setup? Or do we use a multi…"

you can chain multiple service calls asynchronously through Reactor operators like then().. so, the first call will do the necessary setup on the service and then call the next method that you want to test

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

// Arrange
SendChatMessageOptions messageRequest = ChatOptionsProvider.sendMessageOptions();

SendChatMessageResult response = chatThreadClient.sendMessage(messageRequest).block();
Copy link
Member

Choose a reason for hiding this comment

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

Same comment here, need to get rid of this .block() too

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

Copy link
Member

@angiurgiu angiurgiu left a comment

Choose a reason for hiding this comment

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

Looks good now. Thanks for improving the tests!

@minnieliu minnieliu merged commit cf024aa into Azure:master Oct 21, 2020
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