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

Force code coverage for cases where a literal value is returned. #122

Merged
merged 18 commits into from Sep 1, 2019

Conversation

@scouten
Copy link
Collaborator

scouten commented Aug 27, 2019

For now, consider this an experiment meant to provoke discussion about code coverage, rather than something meant to be merged right now.

It's been discussed a few times that code coverage tools do not see lines where a literal value are not flagged as executable lines. To wit:

This is an attempt to fix this problem. I'll start by flagging it as "hacky," but – as a library author – I do want visibility into the numerous branches that would otherwise be hidden by this behavior. (Dare I call it a bug?)

I'm hoping that somebody can point me to a "better" solution that doesn't involve compromising on meaningful code coverage.

As an example, look at https://coveralls.io/builds/25338034/source?filename=lib/xgit/core/dir_cache.ex.

See lines 187, 213, 247, … – All of those are marked as executable and when tested locally via this PR.

Checklist

  • This PR represents a single feature, fix, or change.
  • All applicable changes have been documented. n/a
  • There is test coverage for all changes.
  • Any code ported from jgit maintains all existing copyright and license notices. n/a
  • If new files are ported from jgit, the path to the corresponding file(s) is included in the header comment. n/a
  • Any TO DO items (or similar) have been entered as GitHub issues and the link to that issue has been included in a comment. n/a
scouten added 18 commits Aug 27, 2019
* master:
  Fix typo in changelog. (#139)
  Prepare 0.2.0 release. (#138)
  Update deps-of-dependencies. (#137)
  Upgrade to excoveralls 0.11.2. (#136)
  Update Credo to 1.1.3. (#135)
  Share code for finding working tree in plumbing command modules. (#134)
  [API BREAKING] Merge ValidateObject into Object. (#132)
  Fix up a couple of type specs that should have used FilePath.t. (#131)
  Introduce `stage` type for index file stage references. (#130)
  [API BREAKING] Refactor file path code into new module Xgit.Core.FilePath (#129)
  Introduce a common data type for less-than / equal / greater-than comparisons. (#128)
  Prepare 0.1.6 release. (#127)
  Correct @SPEC for WorkingTree.update_dir_cache/3. (#126)
  Implement Xgit.Plumbing.UpdateIndex.CacheInfo. (#125)
  Fix a couple of bugs I noticed in update_dir_cache. (#124)
  Implement Xgit.Repository.WorkingTree.update_dir_cache/3. (#123)

# Conflicts:
#	lib/xgit/core/dir_cache.ex
Shocker: We have new coverage gaps.
Punt on coverage for the I/O error in mid-file case.
Side benefit: I understand the code (ported from jgit) better now.
@scouten scouten merged commit b341f8b into master Sep 1, 2019
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 100.0%
Details
@scouten scouten deleted the force-coverage branch Sep 1, 2019
@scouten scouten mentioned this pull request Sep 1, 2019
1 of 1 task complete
@scouten scouten self-assigned this Sep 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.