Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

Test Names

testRequirePositive_intIAE1() doesn't tell me anything besides what function is being tested. It is helpful to give some context as to why you think it should throw an exception. What makes testRequirePositive_intIAE1() different from testRequirePositive_intIAE2()? You have to go look at the test to know what is actually being tested. In this case, it is easy to deduce why the argument is invalid, but this will not be the case for more complicated methods.


Repetitiveness

As you pointed out, the test code is bulky and some times hard to follow. When you look at the code, there are lots of cases where each tests is doing the exact same thing, but with different inputs.

This is a perfect case for [parameterized tests][1]parameterized tests. You provide the data with a static method and your test class is instantiated once for each data set.

Touching on the previous point: Parameterized tests mean your actual test method can't describe why the data is intended to cause an error. So it is a good idea to include an additional String argument to provide that context.


Multiple Asserts

testRequireIndexInRangeClosed()1 has three different assertions that are unrelated. These should be different tests (probably different data inputs for a parameterized test). This issue with this is that if the first assertion fails, the following assertions are not checked. This can hide information about what is wrong with the system.

1 This is not the only instance of this in the test code.


Long Class

Just because all of the tested methods are in the same class doesn't mean all of the tests have to be in the same class too. Organize your tests just as you would with your actual code. If a class or method is too long, it likely means you should break it into smaller pieces. [1]: https://stackoverflow.com/a/10143872/210526

Test Names

testRequirePositive_intIAE1() doesn't tell me anything besides what function is being tested. It is helpful to give some context as to why you think it should throw an exception. What makes testRequirePositive_intIAE1() different from testRequirePositive_intIAE2()? You have to go look at the test to know what is actually being tested. In this case, it is easy to deduce why the argument is invalid, but this will not be the case for more complicated methods.


Repetitiveness

As you pointed out, the test code is bulky and some times hard to follow. When you look at the code, there are lots of cases where each tests is doing the exact same thing, but with different inputs.

This is a perfect case for [parameterized tests][1]. You provide the data with a static method and your test class is instantiated once for each data set.

Touching on the previous point: Parameterized tests mean your actual test method can't describe why the data is intended to cause an error. So it is a good idea to include an additional String argument to provide that context.


Multiple Asserts

testRequireIndexInRangeClosed()1 has three different assertions that are unrelated. These should be different tests (probably different data inputs for a parameterized test). This issue with this is that if the first assertion fails, the following assertions are not checked. This can hide information about what is wrong with the system.

1 This is not the only instance of this in the test code.


Long Class

Just because all of the tested methods are in the same class doesn't mean all of the tests have to be in the same class too. Organize your tests just as you would with your actual code. If a class or method is too long, it likely means you should break it into smaller pieces. [1]: https://stackoverflow.com/a/10143872/210526

Test Names

testRequirePositive_intIAE1() doesn't tell me anything besides what function is being tested. It is helpful to give some context as to why you think it should throw an exception. What makes testRequirePositive_intIAE1() different from testRequirePositive_intIAE2()? You have to go look at the test to know what is actually being tested. In this case, it is easy to deduce why the argument is invalid, but this will not be the case for more complicated methods.


Repetitiveness

As you pointed out, the test code is bulky and some times hard to follow. When you look at the code, there are lots of cases where each tests is doing the exact same thing, but with different inputs.

This is a perfect case for parameterized tests. You provide the data with a static method and your test class is instantiated once for each data set.

Touching on the previous point: Parameterized tests mean your actual test method can't describe why the data is intended to cause an error. So it is a good idea to include an additional String argument to provide that context.


Multiple Asserts

testRequireIndexInRangeClosed()1 has three different assertions that are unrelated. These should be different tests (probably different data inputs for a parameterized test). This issue with this is that if the first assertion fails, the following assertions are not checked. This can hide information about what is wrong with the system.

1 This is not the only instance of this in the test code.


Long Class

Just because all of the tested methods are in the same class doesn't mean all of the tests have to be in the same class too. Organize your tests just as you would with your actual code. If a class or method is too long, it likely means you should break it into smaller pieces.

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

Test Names

testRequirePositive_intIAE1() doesn't tell me anything besides what function is being tested. It is helpful to give some context as to why you think it should throw an exception. What makes testRequirePositive_intIAE1() different from testRequirePositive_intIAE2()? You have to go look at the test to know what is actually being tested. In this case, it is easy to deduce why the argument is invalid, but this will not be the case for more complicated methods.


Repetitiveness

As you pointed out, the test code is bulky and some times hard to follow. When you look at the code, there are lots of cases where each tests is doing the exact same thing, but with different inputs.

This is a perfect case for [parameterized tests][1]. You provide the data with a static method and your test class is instantiated once for each data set.

Touching on the previous point: Parameterized tests mean your actual test method can't describe why the data is intended to cause an error. So it is a good idea to include an additional String argument to provide that context.


Multiple Asserts

testRequireIndexInRangeClosed()1 has three different assertions that are unrelated. These should be different tests (probably different data inputs for a parameterized test). This issue with this is that if the first assertion fails, the following assertions are not checked. This can hide information about what is wrong with the system.

1 This is not the only instance of this in the test code.


Long Class

Just because all of the tested methods are in the same class doesn't mean all of the tests have to be in the same class too. Organize your tests just as you would with your actual code. If a class or method is too long, it likely means you should break it into smaller pieces. [1]: http://stackoverflow.com/a/10143872/210526 https://stackoverflow.com/a/10143872/210526

Test Names

testRequirePositive_intIAE1() doesn't tell me anything besides what function is being tested. It is helpful to give some context as to why you think it should throw an exception. What makes testRequirePositive_intIAE1() different from testRequirePositive_intIAE2()? You have to go look at the test to know what is actually being tested. In this case, it is easy to deduce why the argument is invalid, but this will not be the case for more complicated methods.


Repetitiveness

As you pointed out, the test code is bulky and some times hard to follow. When you look at the code, there are lots of cases where each tests is doing the exact same thing, but with different inputs.

This is a perfect case for [parameterized tests][1]. You provide the data with a static method and your test class is instantiated once for each data set.

Touching on the previous point: Parameterized tests mean your actual test method can't describe why the data is intended to cause an error. So it is a good idea to include an additional String argument to provide that context.


Multiple Asserts

testRequireIndexInRangeClosed()1 has three different assertions that are unrelated. These should be different tests (probably different data inputs for a parameterized test). This issue with this is that if the first assertion fails, the following assertions are not checked. This can hide information about what is wrong with the system.

1 This is not the only instance of this in the test code.


Long Class

Just because all of the tested methods are in the same class doesn't mean all of the tests have to be in the same class too. Organize your tests just as you would with your actual code. If a class or method is too long, it likely means you should break it into smaller pieces. [1]: http://stackoverflow.com/a/10143872/210526

Test Names

testRequirePositive_intIAE1() doesn't tell me anything besides what function is being tested. It is helpful to give some context as to why you think it should throw an exception. What makes testRequirePositive_intIAE1() different from testRequirePositive_intIAE2()? You have to go look at the test to know what is actually being tested. In this case, it is easy to deduce why the argument is invalid, but this will not be the case for more complicated methods.


Repetitiveness

As you pointed out, the test code is bulky and some times hard to follow. When you look at the code, there are lots of cases where each tests is doing the exact same thing, but with different inputs.

This is a perfect case for [parameterized tests][1]. You provide the data with a static method and your test class is instantiated once for each data set.

Touching on the previous point: Parameterized tests mean your actual test method can't describe why the data is intended to cause an error. So it is a good idea to include an additional String argument to provide that context.


Multiple Asserts

testRequireIndexInRangeClosed()1 has three different assertions that are unrelated. These should be different tests (probably different data inputs for a parameterized test). This issue with this is that if the first assertion fails, the following assertions are not checked. This can hide information about what is wrong with the system.

1 This is not the only instance of this in the test code.


Long Class

Just because all of the tested methods are in the same class doesn't mean all of the tests have to be in the same class too. Organize your tests just as you would with your actual code. If a class or method is too long, it likely means you should break it into smaller pieces. [1]: https://stackoverflow.com/a/10143872/210526

Source Link
unholysampler
  • 6.2k
  • 19
  • 31

Test Names

testRequirePositive_intIAE1() doesn't tell me anything besides what function is being tested. It is helpful to give some context as to why you think it should throw an exception. What makes testRequirePositive_intIAE1() different from testRequirePositive_intIAE2()? You have to go look at the test to know what is actually being tested. In this case, it is easy to deduce why the argument is invalid, but this will not be the case for more complicated methods.


Repetitiveness

As you pointed out, the test code is bulky and some times hard to follow. When you look at the code, there are lots of cases where each tests is doing the exact same thing, but with different inputs.

This is a perfect case for [parameterized tests][1]. You provide the data with a static method and your test class is instantiated once for each data set.

Touching on the previous point: Parameterized tests mean your actual test method can't describe why the data is intended to cause an error. So it is a good idea to include an additional String argument to provide that context.


Multiple Asserts

testRequireIndexInRangeClosed()1 has three different assertions that are unrelated. These should be different tests (probably different data inputs for a parameterized test). This issue with this is that if the first assertion fails, the following assertions are not checked. This can hide information about what is wrong with the system.

1 This is not the only instance of this in the test code.


Long Class

Just because all of the tested methods are in the same class doesn't mean all of the tests have to be in the same class too. Organize your tests just as you would with your actual code. If a class or method is too long, it likely means you should break it into smaller pieces. [1]: http://stackoverflow.com/a/10143872/210526

lang-java

AltStyle によって変換されたページ (->オリジナル) /