Filter files using a command line flag

[liamappelbe]

Add a command line flag that works like // coverage:ignore-file. It should also be able to handle globs, so that users can ignore all generated files like *.g.dart.

[devoncarew]

I wonder if we should glob *.g.dart by default? Is there value in getting coverage info for generated code? Would excluding it by default be unexpected / surprising for users?

They would have the cli flag to disable the behavior if it was undesired.

[liamappelbe]

I think I would find that surprising, if I was a user. In package:wasm I'd want to disable that behavior. I think the distinction is between users who have generated code that was generated by a 3rd party tool (eg protos), or generated by a tool that is part of the package (which is the case in package:wasm). The former case is probably more common, but in the latter case changing this default behavior would be a confusing breaking change.

As for whether we just add a flag to exclude *.g.dart, or add a flag that excludes arbitrary globs, I think it's probably going to be the same amount of work either way, so I'm inclined to just do the more general arbitrary glob approach.

[natebosch]

I'm in favor of a flag which allows arbitrary globs.

[osa1]

How widely use used is *.g.dart format for generated files? We don't use it in protoc_plugin, we use *.pb.dart.

I agree with @liamappelbe that whether I want to skip generated files or not depends on the use case.

1. If I'm using a code generator (e.g. protoc_plugin) then I want to skip generated files. Testing of generated code should be done by the code generator.
2. If I'm developing a code generator then I want coverage for generated files as well as I'm testing correctness of those in my package.

In general there will be more of (1) than (2) so ignoring generated files might make sense, but we should make sure code generators use a common suffix like .g.dart for generated files. Currently protoc_plugin does not do that (uses .pb.dart). If not then the default will be useless for some of (2) as well.

Either way we should have a flag for arbitrary globs.

If we ignore some files by default then the design of flags gets a bit more complicated as we will probably also need a flag to not ignore anything by default, or have a flag that is opposite of ignore-file (maybe allow-file).

If we don't ignore by default then one flag for ignoring files will do.

I say we don't ignore by default to keep things simple for now. If we want to ignore some files by default in the future it will be the same breaking change when we add it now.

[iapicca]

it would be nice to have a yaml file coverage_options.yaml
(like lints uses analysis_options.yaml)
or even better interact through pubspec.yaml
e.g.:

dev_dependencies:
  coverage: ^100.0.0

coverage:
  ignore:
    - '*.g.dart'
    - '*.pb.dart'

[devoncarew]

How widely use used is *.g.dart format for generated files? We don't use it in protoc_plugin, we use *.pb.dart.

It's the main convention I believe. IIRC the analysis server relies on that file pattern to know to only report one issue for a missing import, instead of hundreds for the rest of the file (for type not found, ...).

If we had a default pattern here, I would default to that one.

Either way we should have a flag for arbitrary globs. I say we don't ignore by default to keep things simple for now. If we want to ignore some files by default in the future it will be the same breaking change when we add it now.

+1

[stact]

Is this enhancement will be planned? It's an awesome idea to have a yml config as @iapicca shared 💯

[liamappelbe]

Is this enhancement will be planned? It's an awesome idea to have a yml config as @iapicca shared 💯

It's not prioritized atm. It's a good first issue though, so contributions are welcome.

[liamappelbe]

dart-archive/coverage#496 added the command line flag. I'll file a separate issue for the yaml config idea.