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 optional disablement for pattern matching else branch #1033

Open
bkuhlmann opened this issue Oct 28, 2022 · 1 comment
Open

Add optional disablement for pattern matching else branch #1033

bkuhlmann opened this issue Oct 28, 2022 · 1 comment

Comments

@bkuhlmann
Copy link

bkuhlmann commented Oct 28, 2022

Overview

Hello. 👋 I'm not sure if this is a bug or a feature request but opting to go with the latter. I'm hitting a situation, with pattern matching, where I don't think I necessarily want code coverage for the else branch when I've configured SimpleCov as follows:

SimpleCov.start do
  enable_coverage :branch
  add_filter %r(^/spec/)
  minimum_coverage_by_file line: 95, branch: 95
end

Consider the following code:

module Demo
  include Dry::Monads[:result]

  def self.call result, logger: Logger.new(STDOUT)
    case result
      in Success(text) then logger.info text
      in Failure(error) then logger.error error
    end
  end
end

I'm using pattern matching and monads for a functional design. Situations in which I'd hit the else branch is low because a monad can only be a Success or Failure. It's not impossible, only very rare to hit the else branch. As an added benefit of pattern matching -- should the else branch be hit -- I'll get a NoMatchingPatternError exception so I'll know immediately if this situation occurs. That last statement makes pattern matching unique versus a traditional case...when statement and why I'm bringing this issue up.

One workaround -- and the most obvious -- is to always implement the else branch:

case result
  in Success(text) then logger.info text
  in Failure(error) then logger.error error
  else logger.fatal("Something unexpected happened.")
end

The other workaround is to ignore code coverage for the else branch entirely:

case result
  in Success(text) then logger.info text
  in Failure(error) then logger.error error
  # :nocov:
end

The only problem with either of these workarounds is that all of this gets tedious quickly for minimum benefit. 😢

Screenshots/Screencasts

Here's a screenshot of the report as generated by SimpleCov:

2022-10-28_10-04-39-Firefox

Steps to Recreate

To recreate, run the following:

  1. Download this tiny demo.zip application.
  2. Run unzip demo.zip.
  3. Change directory to demo.
  4. Run: bundle install
  5. Run: bundle exec rspec
  6. Run: open coverage/index.html

Environment

  • OS: macOS 12.6.1 (21G217)
  • Ruby: ruby 3.1.2p20 (2022-04-12 revision 4491bb740a) [arm64-darwin21.4.0]
  • SimpleCov: 0.21.2
@Morozzzko
Copy link

Ran into the same issue and decided to see if there's a pull request or anything

Running macOS 14.2.1
ruby 3.2.3 (2024-01-18 revision 52bb2ac0a6) [x86_64-darwin23]
simpleov 0.22.0

My system shows ≈90%+ line coverage, but just ≈75% branch coverage. I was fairly surprised because I do test all significant branches, and those untested were mostly from generated boilerplate and weren't that important to test.
As it turns out, nearly all of those were from else branches in pattern matching

The thing is: we shouldn't care for an else branch for PM unless it's explicitly defined. A skilled developer would probably try and use PM with a comprehensive coverage – meaning there's no valid data that must be handled explicitly.
Writing tests for an absurd situation isn't particularly useful

So I'd like to have an option to handle that, too

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

No branches or pull requests

2 participants