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

Add unit tests for com.zheng.common.util.Paginator #2

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

EdgarScott
Copy link

I've analysed your codebase and noticed that com.zheng.common.util.Paginator is not fully tested.

I've written some tests for the methods in this class with the help of Diffblue Cover.

Hopefully, these tests will help you detect any regressions caused by future code changes. If you would find it useful to have additional tests written for this repository, I would be more than happy to look at other classes that you consider important in a subsequent PR.

Scott Buchanan added 2 commits July 30, 2019 09:55
These tests were written using Diffblue Cover.
These tests were written using Diffblue Cover.
@EdgarScott
Copy link
Author

Updated as per Peter's feedback:

  1. Removed tests for basic set/get functions
  2. Changed name from objectUnderTest to the actual object name

@EdgarScott EdgarScott requested a review from petersear July 30, 2019 14:06
Copy link

@petersear petersear left a comment

Choose a reason for hiding this comment

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

Formatting looks good.

@PowerMockIgnore({"javax.management.*"})
public class PaginatorTest {

/* testedClasses: Paginator */

Choose a reason for hiding this comment

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

Remove comment


/* testedClasses: Paginator */
@Test
public void initTotalPageOutputVoid() {

Choose a reason for hiding this comment

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

This test provides 100% coverage for the method :-)
I wonder if we could use some of the setters so that the expected value is non-zero?

paginator.setParam(null);
paginator.setPage(0);
paginator.setTotalPage(0L);
Reflector.setField(paginator, "rows", 2);

Choose a reason for hiding this comment

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

Reflection

InvocationTargetException {
PowerMockito.mockStatic(Math.class);
final Paginator paginator = new Paginator();
Reflector.setField(paginator, "total", 0L);

Choose a reason for hiding this comment

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

We don't want to see reflection in our tests. We might be able to work around this using constructor/getters/setters.

InvocationTargetException {
PowerMockito.mockStatic(Math.class);
final Paginator paginator = new Paginator();
Reflector.setField(paginator, "total", 0L);

Choose a reason for hiding this comment

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

Reflection

paginator.setParam("foo");
paginator.setPage(0);
paginator.setTotalPage(0L);
Reflector.setField(paginator, "rows", -786_432);

Choose a reason for hiding this comment

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

Reflection

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.

2 participants