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

BlackBoxBoundedContext improvements #1146

Merged
merged 37 commits into from Aug 29, 2019
Merged

BlackBoxBoundedContext improvements #1146

merged 37 commits into from Aug 29, 2019

Conversation

dmdashenkov
Copy link
Contributor

@dmdashenkov dmdashenkov commented Aug 27, 2019

In this PR we provide a number of enhancements for the BlackBoxBoundedContext API:

  1. The Truth API is updated to version 1.0.
  2. Custom Subjects now expose the actual() method in order to allow users to build more complex verifications, for example, to test metadata of dispatched signals.

@dmdashenkov dmdashenkov self-assigned this Aug 27, 2019
@dmdashenkov dmdashenkov changed the title Bbbc improvements BlackBoxBoundedContext improvements Aug 27, 2019
@dmdashenkov dmdashenkov marked this pull request as ready for review August 29, 2019 10:19
@dmdashenkov
Copy link
Contributor Author

@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.

@dmdashenkov please see my comments.

AggUserNotified.class))
.close();
EventSubject assertEvents = context.receivesCommand(reassignTask())
.assertEvents();
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks the alignment is broken here.

}
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

This line looks redundant to me.


MESSAGE_COUNT("the count of the generated messages is"),
REQUESTED_INDEX("but the requested index was"),
@SuppressWarnings("DuplicateStringLiteralInspection")
Copy link
Contributor

Choose a reason for hiding this comment

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

Please comment on this. Just for consistency.

if (index >= size) {
public final ProtoSubject message(int index) {
if (messages() == null) {
failWithActual(fact(FactKey.ACTUAL.value, null));
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use the enum values without referencing the enum name?

T outerObject = Iterables.get(messages(), index);
Message unpacked = AnyPacker.unpack(outerObject.getMessage());
return ProtoTruth.assertThat(unpacked);

Copy link
Contributor

Choose a reason for hiding this comment

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

Please kill this line.

*
* @return an immutable copy of the {@code actual} messages
*/
public List<T> actual() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure, but we tend to use the returning values of such methods as plain Lists and then eventually wrap them into ImmutableLists over and over again.

So if we can declare the ImmutableList as a type of the returning value, I would do so.

@codecov
Copy link

codecov bot commented Aug 29, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@8614e6e). Click here to learn what that means.
The diff coverage is 81.73%.

@@            Coverage Diff            @@
##             master    #1146   +/-   ##
=========================================
  Coverage          ?   91.72%           
  Complexity        ?     4094           
=========================================
  Files             ?      536           
  Lines             ?    12968           
  Branches          ?      748           
=========================================
  Hits              ?    11895           
  Misses            ?      839           
  Partials          ?      234

@dmdashenkov
Copy link
Contributor Author

@armiol, PTAL again.

@armiol
Copy link
Contributor

armiol commented Aug 29, 2019

@dmdashenkov @dmitrykuzmin please settle down on the conflicts. We need 1.0.2 to release the Bootstrap in 1.0.2.

@dmdashenkov dmdashenkov merged commit 2a73509 into master Aug 29, 2019
@dmdashenkov dmdashenkov deleted the bbbc-improvements branch August 29, 2019 19:11
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.

None yet

2 participants