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

Added Branch Coverage Tool & Unit Tests for parseEntry() #2

Merged
merged 15 commits into from
Feb 18, 2020

Conversation

simonsirak
Copy link
Owner

Added

  • Branch coverage tool for GvkParser::parseEntry(). The tool considered if, else (and if one was missing, it was inserted), for (and skipped for loops). It does not consider nested if statements that are specifically a result of short-circuiting. No ternary operators or try/catch were encountered. The tool has results that are relatively consistent with the automated tool for this project, Jacoco.
  • 4 cases that increase the branch coverage from ~75% to ~84% using our coverage tool.

Increases manual branch coverage by ~5 percentage points.
This simplifies test inspection in the
future.
Add final test cases and simplify
the existing test cases. Now, they
are properly based on the PICA spec.
Copy link
Collaborator

@yiiju yiiju left a comment

Choose a reason for hiding this comment

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

Looks good 👍

Copy link
Collaborator

@christinerosquist christinerosquist left a comment

Choose a reason for hiding this comment

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

Nice job 👍

Copy link
Collaborator

@SoFoDa SoFoDa left a comment

Choose a reason for hiding this comment

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

parseEntry() seems to be a good candidate in terms of refactoring. We could refactor each tag procedure into smaller functions before merging.

simonsirak and others added 6 commits February 17, 2020 10:55
yiiju and others added 4 commits February 17, 2020 20:55
Now it is adjusted based on the refactoring
Change names to same structure
@simonsirak simonsirak merged commit ec59705 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