Skip to content

Added branch coverage tool and unit tests for Authors::handleArgument#3

Merged
yiiju merged 7 commits into
masterfrom
handleArgument
Feb 18, 2020
Merged

Added branch coverage tool and unit tests for Authors::handleArgument#3
yiiju merged 7 commits into
masterfrom
handleArgument

Conversation

@yiiju

@yiiju yiiju commented Feb 15, 2020

Copy link
Copy Markdown
Collaborator

Added

  • Branch coverage tool for Authors::handleArgument.
  • Add 4 new unit tests that increase the branch coverage from 58% to 73% using our coverage tool.
  • Add refactored Authors::handleArgument function into a new file called AuthorsRefactor and unit tests file called AuthorsRefactorTest to reduce cyclomatic complexity from 36 to 7.

@yiiju yiiju changed the title Handle argument Added branch coverage tool and unit tests for Authors::handleArgument Feb 15, 2020

@simonsirak simonsirak left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Looks good! 👍Had a couple of questions but nothing crucial. I pushed some modifications I had made to our tool, since it had some bugs. It should still work fine though, hopefully ^^

Comment thread src/test/java/org/jabref/logic/layout/format/AuthorsTest.java Outdated
Comment thread src/test/java/org/jabref/logic/layout/format/AuthorsTest.java Outdated

@simonsirak simonsirak left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Looks good :)

@yiiju yiiju requested a review from simonsirak February 15, 2020 23:33

@simonsirak simonsirak left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Looks good!

@christinerosquist christinerosquist left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good! Nice job, I approve 👍

@yiiju yiiju merged commit 9330e7e into master Feb 18, 2020
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.

4 participants