Skip to content
This repository has been archived by the owner on Oct 13, 2020. It is now read-only.

A new Maven module 'matchers' in 'junit.contrib' started with 'IsThrowable' matcher. #7

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Tibor17
Copy link

@Tibor17 Tibor17 commented Oct 4, 2011

Hi All,

I would like to kindly ask you to review my set of proposed code changes in a new module called matchers in the junit.contrib.

I have completed the work with javadoc in the class named IsThrowable, where you can find the purpose of the first matcher in this module, useful matcher's methods in tests, and use cases. The test TestIsThrowable contains a set of test cases which also demonstrate the use.

Thx, Tibor

matchers/pom.xml Outdated
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<version>4.9</version>
Copy link
Member

Choose a reason for hiding this comment

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

4.10 is live.

@Tibor17
Copy link
Author

Tibor17 commented Oct 10, 2011

The Javadoc is now composed of three parts, namely behaviour, use cases and known limitations.

It covers all possibilities of use, that's the reason why multiple calls to IBlock are there as well. Although the tests exist also for the multiple calls, I can remove this narrow part of the javadoc. A javadoc must exist in every class.

But the question is if the API (mainly signatures of static method) in this matcher are comprehensive, or I should make any additional modifications.

* has four variants of matchers, where the last two are able to put the stack trace from the failing block to a
* description (output) stream:
* <p><blockquote><pre>
* throwing(Class<a><</a><a>? </a>extends Throwable<a>></a>... ): Matcher<a><</a>IsThrowable.IBlock<a>></a>
Copy link
Member

Choose a reason for hiding this comment

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

Tibor,

It seems now from examining github that a comment I attempted to make here previously didn't actually get posted, which has likely meant that my subsequent comments have not made as much sense as they could.

I would prefer that we choose an API that separates the idea of "catch what this block throws" from that of "assert things about the caught exception". Thus, I'm imagining a single throwing method, with a different type signature:

Matcher<IsThrowable.IBlock> throwing(Matcher<? extends Throwable> matcher);

Thus,

throwing(X) becomes throwing(isA(X))
notThrowing(X) becomes throwing(not(isA(X)))
throwingDeeply(X) becomes throwing(deeply(X)) // this one I'm less sure about.

This has many advantages. For starters, it collapses the size of the API. Secondly, it makes it easy to chain assertions about a single thrown exception:

throwing(both(isA(NullPointerException.class)).and(hasMessage("everything failed!")))

What do you think?

@Tibor17
Copy link
Author

Tibor17 commented Oct 10, 2011

Hi David,

I was thinking initially of this concept as well. And there can be a nice use of combinations like:

throwing(anyOf(exception))

but mistakenly the developer may instead type throwing(allOf(exceptions)) which is zero intersection because two exceptions can not be thrown at the same time.

Originally the message was optional, which should be again kept in mind in the concept design.

Let me think of it completely.
Now I want to submit long waiting changes for the issue #307 in the junit, and then i'll come back here.

@dsaff
Copy link
Member

dsaff commented Oct 11, 2011

Okay, I'll look forward to your return. :-)

… method parameter.

Additionally IsThrowing, IsThrowable, IsRegex, IsRegexResult, IsAssignable, and negative+positive tests.
@Tibor17
Copy link
Author

Tibor17 commented Feb 5, 2012

i've done my job. Pls see the tests to understand the use. There are a lot of them, but the forms like this are possible as required
throwing(both(isA(NullPointerException.class)).and(hasMessage("everything failed!")))

@Tibor17
Copy link
Author

Tibor17 commented Feb 17, 2012

My commitment was broken, so i will today repair the previous commit.

@Tibor17
Copy link
Author

Tibor17 commented May 14, 2012

@dsaff

David, would you

mvn clean test

with this repo?

It should launch all tests successfully.

I believe in JUnit we may additionally extend the ExpectedException using this.

@dsaff
Copy link
Member

dsaff commented May 14, 2012

Tibor,

I'm pretty busy for the next couple of days, but I'll try to get to it soon. Thanks,

David

@Tibor17
Copy link
Author

Tibor17 commented Jul 18, 2012

@dsaff
David, what's the cost to get our junit.contrib module on maven repository?
I would be lucky if we could have it there.

this.fallback = fallback;
}

abstract V call1() throws Exception;
Copy link
Member

Choose a reason for hiding this comment

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

Why call1?

Copy link
Author

Choose a reason for hiding this comment

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

It is because this abstract private package method is used only by matcher.
The abstract class should not have an access to run().
The call1() calls either run() or call() depending on implementation Block/Callable.

Then the evaluate() evaluates call1(), and toString() returns actual status of invoked block on call1.

@dsaff
Copy link
Member

dsaff commented Jul 18, 2012

Tibor,

Before moving forward, it comes to mind that most of this could be contributed directly to hamcrest, which would give it more visibility, likely, than junit.contrib. Have you considered that? Thanks,

David

@Tibor17
Copy link
Author

Tibor17 commented Jul 18, 2012

@dsaff
Yes Hamcrest makes sense to me as well, i will definitely try to contribute.
But contributing to Junit and ExpectedException API would mostly make sense as final destination of use.
At least I would like to know how the people would see the use of my matchers in ExpectedException#expect(), and let them try out. Are these matchers comfortable, ...

Meanwhile i can contribute to Hamcrest, but synchronizing a new release of Hamcrest with Junit release on these Matchers is not completely decided by me.

@marcphilipp marcphilipp changed the base branch from master to main June 15, 2020 17:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants