Skip to content

add hamcrest-library as a test dependency #1301

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

Merged
merged 2 commits into from
Jun 23, 2016

Conversation

alb-i986
Copy link
Contributor

As discussed in #1292
(see #1292 (comment))

@kcooney
Copy link
Member

kcooney commented May 18, 2016

Do we have immediate plans to use this dependency?

@alb-i986
Copy link
Contributor Author

I may use it here

https://github.com/junit-team/junit4/pull/1302/files#diff-5ac6ee0460714bbb4ceac9a9b91df336R39

to ensure that the signature of failureCountIs allows to take greaterThan.

@kcooney
Copy link
Member

kcooney commented May 18, 2016

I realize this is personal preference and I am looking at only two data points , but I think that 69e8a8a made the tests easier to read. It is extremely rare for me to write an assertion that a given integer is greater that some value (or less than some value, or not equal to some value).

That being said, I find Hamcrest assertions hard to read (compared with Fest or Truth) so it might be just me, so I won't object to the new dependency.

@alb-i986
Copy link
Contributor Author

I find Hamcrest assertions hard to read (compared with Fest or Truth) so it might be just me, so I won't object to the new dependency.

Sounds like you are not happy with hamcrest. Fair enough, but here I'm not proposing to introduce hamcrest, I'm only trying to help us contributors writing cleaner unit tests by providing more Matchers to use. It's only about giving more options, really.

Besides greaterThan and similar, another pretty common matcher which is missing right now is hasSize.

$ rgrep 'assertThat.*[sS]ize' src/test/java/
src/test/java/org//junit/experimental/categories/CategoryValidatorTest.java:        assertThat(errors.size(), is(1));
src/test/java/org//junit/experimental/categories/CategoryValidatorTest.java:        assertThat(errors.size(), is(0));
src/test/java/org//junit/internal/runners/ErrorReportingRunnerTest.java:        assertThat(firedFailures.size(), is(1));
src/test/java/org//junit/runners/model/TestClassTest.java:        assertThat("Wrong number of annotated fields.", annotatedFields.size(), is(3));
src/test/java/org//junit/runners/model/TestClassTest.java:        assertThat(values.size(), is(1));
src/test/java/org//junit/runners/model/TestClassTest.java:      assertThat(values.size(), is(1));
src/test/java/org//junit/tests/experimental/max/DescriptionTest.java:        assertThat(description.getAnnotations().size(), equalTo(0));
src/test/java/org//junit/tests/experimental/max/DescriptionTest.java:        assertThat(description.getAnnotations().size(), equalTo(1));
src/test/java/org//junit/tests/experimental/max/DescriptionTest.java:        assertThat(description.getAnnotations().size(), equalTo(0));
src/test/java/org//junit/tests/experimental/theories/internal/AllMembersSupplierTest.java:        assertThat(valueSources.size(), is(2));
src/test/java/org//junit/tests/experimental/theories/internal/AllMembersSupplierTest.java:        assertThat(valueSources.size(), is(2));
src/test/java/org//junit/tests/experimental/theories/runner/SuccessfulWithDataPointFields.java:            assertThat(list.size(), is(1));
src/test/java/org//junit/tests/experimental/theories/runner/WithDataPointMethod.java:            assertThat(first.size(), is(0));
src/test/java/org//junit/validator/AnnotationsValidatorTest.java:        assertThat(errors.size(), is(1));
src/test/java/org//junit/validator/PublicClassValidatorTest.java:        assertThat("Wrong number of errors.", validationErrors.size(),

All of these assertions could be refactored to use hasSize.

@kcooney
Copy link
Member

kcooney commented Jun 18, 2016

I'll defer to Marc on this one.

@marcphilipp marcphilipp merged commit 8fb8153 into junit-team:master Jun 23, 2016
@marcphilipp
Copy link
Member

Thanks!

@alb-i986 alb-i986 deleted the add-hamcrest-library-test-dep branch June 23, 2016 20:01
@alb-i986
Copy link
Contributor Author

Thank you @marcphilipp!

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