cenfun / monocart-coverage-reports

A code coverage tool to generate native V8 reports or Istanbul reports.
MIT License
31 stars 5 forks source link

[Feature Request] Add `addFromDir` option #51

Closed mrazauskas closed 2 months ago

mrazauskas commented 2 months ago

Currently .addFromDir() is method, but I would like to use it as an option. If that isn’t complicated to implement.

For instance, inputDir is somewhat similar option, so I though why not to have addFromDir as an option too? Also cleanCache or clean can be used as options or as methods.


I had two separate scripts to generate raw report and to merge coverage into codacy report. Since the codacy report was added, I merged both scripts into one. Now looking at the file it seems like addFromDir could be an option too. See this line: https://github.com/tstyche/tstyche/blob/8e16c416dc614226974bf8be29ab884fcf6b51e7/scripts/report-coverage.js#L53

cenfun commented 2 months ago

Yes, it's good idea to provide a option for .addFromDir() which is just a string type. Before do that let‘s consider how to name this option?

So currently we already have:

To avoid confusing the users, what is name of this option for raw v8 coverage json files?

What's your idea?

Additional explanation for the different data:

mrazauskas commented 2 months ago

What if inputDir would be able to handle both types of data? The name of inputDir is so abstract. This can be any supported input data. Feels intuitive.

The input data type could be specified like this:

{
  inputDir: ['./v8-coverage', { type: 'v8' }]
}

{
  inputDir: ['./raw-coverage-unit', './raw-coverage-e2e', { type: 'raw' }]
}

// 'raw' is the default and can be omitted, so all works just like now
{
  inputDir: ['./raw-coverage-unit', './raw-coverage-e2e']
}

I think, the type must be specified for inputDir, not for each path. Otherwise config can get too complex to interpret.

mrazauskas commented 2 months ago

Alternatively input data type could be simply specified via some inputType option:

{
  inputDir: './v8-coverage',
  inputType: 'v8'
}
cenfun commented 2 months ago

Yes, your approach should work, but as you mentioned, it requires an additional specification of type. I personally think using a new option name might be better.

cenfun commented 2 months ago

I prefer to dataDir

mrazauskas commented 2 months ago

Sounds good too. Indeed specifying type is somewhat clumsy.

cenfun commented 2 months ago

please try new version 2.9.0 for new option dataDir

mrazauskas commented 2 months ago

Thanks! All works as expected.