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

Encapsulate invocations inside cardinality and handle Expectaction#throws scenario #410

Closed
wants to merge 15 commits into from

Conversation

floehopper
Copy link
Member

@floehopper floehopper commented Nov 8, 2019

This PR includes the 2nd set of commits from #394. The 1st set were merged in 00f0540.

@nitishr was the original author of all the commits in this branch. The original motivation came from #178.

Cardinality and invocations go hand-in-hand. So, the target is to move
invocations to Cardinality.
Cardinality and invocations go hand-in-hand. So, the target is to move
invocations to Cardinality.
The target is to encapsulate it inside Cardinality in a tell-don't-ask
style.
The target is to encapsulate it inside Cardinality in a tell-don't-ask
style.
expected hadn't quite sounded right because it also included allowed.
Using more expressive & concise conditionals & booleans.
This helps us avoid rescuing an exception raised by code elsewhere in
the Mocha code. Also, enables us to handle the missed scenario where
Expectaction#throws is called, 'coz without it, we'd need a generic way
to catch any thrown tag and that might be quite awkward.
Copy link
Member Author

@floehopper floehopper left a comment

Choose a reason for hiding this comment

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

@nitishr Sorry for the delay. I've now reviewed this. It looks good to me and I plan to fix-up one minor thing myself and then get it merged. Thanks for your work on it and your patience in waiting for me to review it.

Since I'm the "author" of this PR, I'm not allowed to approve it, so I'm just adding this as a comment. However, this won't prevent me from merging it.

test/unit/cardinality_test.rb Show resolved Hide resolved
test/acceptance/exception_rescue_test.rb Show resolved Hide resolved
floehopper added a commit that referenced this pull request Nov 16, 2019
Also handles Expectaction#throws scenario.

PR: #410

Second bunch of commits from #394.

Co-authored-by: Nitish Rathi <[email protected]>
@floehopper
Copy link
Member Author

Merged in 4470825.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants