very_good_cli icon indicating copy to clipboard operation
very_good_cli copied to clipboard

feat(test): support capturing coverage from all files

Open felangel opened this issue 2 years ago • 7 comments

As a developer, I want to be able to run tests and have the reported coverage be absolute rather than just based on the imported paths so that I can have a wholistic overview of the level of test coverage.

Currently, if a file is not imported as part of a test it will never be reported via coverage and will not be reflected in the overall coverage metrics.

Proposal

Introduce a --collect-coverage-from option that will allow developers to control how coverage is collected.

Options

  • imports (default): collect coverage from imported files only
  • all: collect coverage from all files
# Run tests and require 100% coverage across all files
very_good test --min-coverage 100 --collect-coverage-from all

felangel avatar Jun 02 '22 16:06 felangel

Enums of two values can be like booleans. Cant that be a flag instead of an option? Then we have possibility of abbr for an even shorter syntax?

suggestions

--collect-coverage-imports-only (default true)

# or

--collect-coverage-from-all (default false)

I personally like flags that default to false since their absence is more intuitive to be false

renancaraujo avatar Jun 02 '22 16:06 renancaraujo

--collect-coverage-from-all (default false)

Yeah I'm fine with that 👍

I just wasn't sure if we'd potentially support other variants in the future but at the moment I can't think of anything besides maybe:

--collect-coverage-from path ./some/custom/path/**

felangel avatar Jun 02 '22 16:06 felangel

What would be the use case for only wanting import coverage, and not all? The way I see it, my initial reaction was that this was a bug in the test command in Dart, and not a design feature. That's why I can only see value at the moment of having all as the only option really that makes sense.

jorgecoca avatar Jun 02 '22 21:06 jorgecoca

What would be the use case for only wanting import coverage, and not all? The way I see it, my initial reaction was that this was a bug in the test command in Dart, and not a design feature. That's why I can only see value at the moment of having all as the only option really that makes sense.

That makes it easy haha however it would technically be a breaking change.

felangel avatar Jun 03 '22 03:06 felangel

I like:

--collect-coverage-from-all (default false)

However, I agree with @jorgecoca feels that the default behaviour should be collect-coverage-from-all. Perhaps, a breaking change can be made, if we decide so we can provide --collect-coverage-imports-only for easier migration, now defaulting to false, (cc: @renancaraujo ).

alestiago avatar Jun 03 '22 10:06 alestiago

@alestiago Agreed! As long it is a flag defaulting to false, either is ok for me.

renancaraujo avatar Jun 03 '22 12:06 renancaraujo

As a workaround for this, I wrote a dart script that programmatically generate a "coverage_helper_test.dart" that imports all project's dart files before running the tests. As a result, it collects coverage from all files, even those files we are not testing, and help us understand how tested a project is.

I'm not sure if it's what you or the community is looking for, but works fine. Perhaps the same algorithm could be used when --collect-coverage-from-all flag is used.

micaelcid avatar Jun 23 '22 14:06 micaelcid