Skip to content

Send external events from black box tests#903

Merged
mdrachuk merged 25 commits intomasterfrom
black-box-external-events
Dec 5, 2018
Merged

Send external events from black box tests#903
mdrachuk merged 25 commits intomasterfrom
black-box-external-events

Conversation

@mdrachuk
Copy link
Contributor

@mdrachuk mdrachuk commented Dec 5, 2018

This PR adds a receivesExternalEvent call to the BlackBoxBoundedContext.

@codecov
Copy link

codecov bot commented Dec 5, 2018

Codecov Report

Merging #903 into master will increase coverage by 0.02%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master     #903      +/-   ##
============================================
+ Coverage     93.19%   93.22%   +0.02%     
- Complexity     3816     3823       +7     
============================================
  Files           522      522              
  Lines         12573    12585      +12     
  Branches        711      710       -1     
============================================
+ Hits          11718    11732      +14     
+ Misses          626      625       -1     
+ Partials        229      228       -1

…events

# Conflicts:
#	testutil-client/src/main/java/io/spine/testing/client/blackbox/ErrorAbsenceVerify.java
#	testutil-server/src/main/java/io/spine/testing/server/blackbox/BlackBoxBoundedContext.java
#	testutil-server/src/test/java/io/spine/testing/server/blackbox/BlackBoxBoundedContextTest.java
#	testutil-server/src/test/java/io/spine/testing/server/blackbox/given/BbProjectAggregate.java
#	testutil-server/src/test/java/io/spine/testing/server/blackbox/given/Given.java
@mdrachuk mdrachuk requested a review from armiol December 5, 2018 14:02
@mdrachuk mdrachuk self-assigned this Dec 5, 2018
@mdrachuk
Copy link
Contributor Author

mdrachuk commented Dec 5, 2018

@armiol, PTAL.

…events

# Conflicts:
#	testutil-server/src/main/java/io/spine/testing/server/blackbox/BlackBoxBoundedContext.java
#	testutil-server/src/test/java/io/spine/testing/server/blackbox/BlackBoxBoundedContextTest.java
Copy link
Contributor

@armiol armiol left a comment

Choose a reason for hiding this comment

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

@mdrachuk please see my comments.

* @apiNote Returned value can be ignored when this method invoked for test setup
*/
@CanIgnoreReturnValue
public T receivesExternalEvent(Message messageOrEvent) {
Copy link
Contributor

Choose a reason for hiding this comment

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

In case of a plain message posted, it's not clear how to set the mandatory ExternalMessage.bounded_context_name field.

@VisibleForTesting
final class BlackBoxSetup {

private final BoundedContextName contextName;
Copy link
Contributor

Choose a reason for hiding this comment

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

If the one above resolved, this field requires (maybe) both renaming and explanation OR removal.

}

@React(external = true)
Optional<BbUserUnassigned> on(BbUserDeleted event) {
Copy link
Contributor

Choose a reason for hiding this comment

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

BbUserUnassigned is about a user, while we speak of a Project aggregate. So probably this is BbAssigneeRemoved.

}

//
// External User bounded context events
Copy link
Contributor

Choose a reason for hiding this comment

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

`Users`

// An external event to be imported into an aggretate.
// An event to be imported into an aggretate.
//
message TuCommentRecievedByEmail {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's fix the spelling here as well. TuCommentReceivedByEmail

repeated BbProjectId project_id = 2;
}

message BbAddProjectAssignee {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
message BbAddProjectAssignee {
message BbAssignProject {

@mdrachuk
Copy link
Contributor Author

mdrachuk commented Dec 5, 2018

@armiol, PTAL.

Copy link
Contributor

@armiol armiol left a comment

Choose a reason for hiding this comment

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

@mdrachuk LGTM with a minor documentation-related comment to address.

* optional external events to be dispatched to the Bounded Context
* in supplied order
* @return current instance
* @apiNote Returned value can be ignored when this method invoked for test setup
Copy link
Contributor

Choose a reason for hiding this comment

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

We have agreed to put periods in @apiNote and @implNote, as these are sentences.

@mdrachuk mdrachuk merged commit 1e40535 into master Dec 5, 2018
@mdrachuk mdrachuk deleted the black-box-external-events branch December 5, 2018 18:44
@armiol armiol mentioned this pull request Dec 20, 2018
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.

2 participants