danmayer / coverband

Ruby production code coverage collection and reporting (line of code usage)
https://github.com/danmayer/coverband
MIT License
2.5k stars 161 forks source link

Documentation on Ignoring Files states regex but uses wildcards, and regexes are #464

Closed hlascelles closed 1 year ago

hlascelles commented 1 year ago

Describe the bug The documentation on Ignoring Files states regex but uses wildcards: https://github.com/danmayer/coverband/tree/5f0d8fe754336a0c53d8bd80e6d947875e0efa8b#ignoring-files

image

In the example above (2) it is not matching "any string after the slash", it is matching "any number of slashes".

Example error

The docs example above is wrong, but "harmless", it will just give the wrong results. But consider this:

  config.ignore += %w[
    *foo*
  ]

In this case, the regex is faulty. The first asterisk needs a matching character before it. If you hit the report page you will get a 500 error: RegexpError: target of repeat operator is not specified: /*foo*/. Note this happens at later "report" runtime, not boot time.

To fix

Up to @danmayer as to what to do here. Is it:

  1. A simple docs change to show right regexes
  2. A bit more work to check the ignore at boot time to make sure they are all valid regexes. ie fail fast :+1:
  3. Only allow regexes
  4. Something else
  5. Me misunderstanding the example (possible! :) )
danmayer commented 1 year ago

yeah, good callout... The code that uses this is like so [https://github.com/danmayer/coverband/blob/main/lib/coverband/collectors/delta.rb#L57](delta using ignores).

          next unless @@ignore_patterns.none? { |pattern| file.match(pattern) } &&
            file.start_with?(@@project_directory)

So it isn't just the error in the reporting UI, it can fail to store coverage with invalid regex...

I think you are right on checking the ignores as they are set and failing fast... Let me take a look at supporting that.

danmayer commented 1 year ago

thanks for the report @hosamaly take a look at the solution and let me know if that seems like it would help users.

hlascelles commented 1 year ago

Looks good to me. (That comment ^^^ may have been to me, not @hosamaly, unless he is a committer/admin).