-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Parameterized reuses TestClass #1449
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
Changes from all commits
9d2fe4e
de3404d
efdcb37
f34c443
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
package org.junit.internal; | ||
|
||
/** @since 4.13 */ | ||
public final class Checks { | ||
|
||
private Checks() {} | ||
|
||
/** | ||
* Checks that the given value is not {@code null}. | ||
* | ||
* @param value object reference to check | ||
* @return the passed-in value, if not {@code null} | ||
* @throws NullPointerException if {@code value} is {@code null} | ||
*/ | ||
public static <T> T notNull(T value) { | ||
if (value == null) { | ||
throw new NullPointerException(); | ||
} | ||
return value; | ||
} | ||
|
||
/** | ||
* Checks that the given value is not {@code null}, using the given message | ||
* as the exception message if an exception is thrown. | ||
* | ||
* @param value object reference to check | ||
* @param message message to use if {@code value} is {@code null} | ||
* @return the passed-in value, if not {@code null} | ||
* @throws NullPointerException if {@code value} is {@code null} | ||
*/ | ||
public static <T> T notNull(T value, String message) { | ||
if (value == null) { | ||
throw new NullPointerException(message); | ||
} | ||
return value; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,12 @@ | ||
package org.junit.internal.builders; | ||
|
||
import org.junit.runner.Runner; | ||
import org.junit.runners.BlockJUnit4ClassRunner; | ||
import org.junit.runners.JUnit4; | ||
import org.junit.runners.model.RunnerBuilder; | ||
|
||
public class JUnit4Builder extends RunnerBuilder { | ||
@Override | ||
public Runner runnerForClass(Class<?> testClass) throws Throwable { | ||
return new BlockJUnit4ClassRunner(testClass); | ||
return new JUnit4(testClass); | ||
} | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change is unrelated to the reuse of TestClass. It should be a separate commit. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is a separate commit. See 11822c4 I have this commit in this pull since there's no reason for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, my fault. I was not aware that the PR is made up of multiple commits. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
package org.junit.internal; | ||
|
||
import static org.junit.Assert.assertEquals; | ||
import static org.junit.Assert.assertNull; | ||
import static org.junit.Assert.assertSame; | ||
import static org.junit.Assert.fail; | ||
import static org.junit.internal.Checks.notNull; | ||
import org.junit.Test; | ||
|
||
/** Tests for {@link Checks}. */ | ||
public class ChecksTest { | ||
|
||
@Test | ||
public void notNullShouldReturnNonNullValues() { | ||
Double value = Double.valueOf(3.14); | ||
|
||
Double result = notNull(value); | ||
|
||
assertSame(value, result); | ||
} | ||
|
||
@Test | ||
public void notNullShouldThrowOnNullValues() { | ||
try { | ||
notNull(null); | ||
fail("NullPointerException expected"); | ||
} catch (NullPointerException e) { | ||
assertNull("message should be null", e.getMessage()); | ||
} | ||
} | ||
|
||
@Test | ||
public void notNullWithMessageShouldReturnNonNullValues() { | ||
Float value = Float.valueOf(3.14f); | ||
|
||
Float result = notNull(value, "woops"); | ||
|
||
assertSame(value, result); | ||
} | ||
|
||
@Test | ||
public void notNullWithMessageShouldThrowOnNullValues() { | ||
try { | ||
notNull(null, "woops"); | ||
fail("NullPointerException expected"); | ||
} catch (NullPointerException e) { | ||
assertEquals("message does not match", "woops", e.getMessage()); | ||
} | ||
} | ||
|
||
@Test | ||
public void notNullWithNullMessageShouldThrowOnNullValues() { | ||
try { | ||
notNull(null, null); | ||
fail("NullPointerException expected"); | ||
} catch (NullPointerException e) { | ||
assertNull("message should be null", e.getMessage()); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should not extract a Checks class and simply inline the validations, in order to not provide methods someone may use and depend on. I know that is a very weak argument. I leave it up to you whether you want to keep it or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As you can see in the commits, we have multiple classes that do
null
checks. Quite often when I write code in JUnit I find myself wanting a one-liner to check for null.I doubt anyone would depend on this class, since 1) it's in an internal package, 2) it has an odd name, and 3) most everyone that is using JDK 6 or later would use
Objects.requireNotNull()
or Guava'sPreconditions.checkNotNull()
.