Support "additionalEntries" option in @cypress/webpack-dev-server (like @cypress/webpack-preprocessor)

[toaditoad]

What would you like?

I would like @cypress/webpack-dev-server to support "additionalEntries" option to bundle additional source files when executing component tests with Cypress 7.x. See example on https://github.com/toaditoad/code-coverage-mwe/blob/main/tests/cypress/plugins/index.ts that uses @cypress/webpack-preprocessor and Cypress 6.x.

Why is this needed?

I used to generate code coverage by leveraging the "additionalEntries" option of @cypress/webpack-preprocessor (with Cypress 6.9.1). This was necessary to report 0% coverage for source files that have not been touched by tests. See https://github.com/toaditoad/code-coverage-mwe for a now working minimal example.

[lmiller1990]

I think it makes sense for our dev server to support a similar API to the preprocessor.

I could be missing something, but isn't it implied a file that is not bundled has 0% code coverage? What is the benefit of including it in the bundle if it will always be at 0% (not used).

[toaditoad]

Sorry, let me rephrase:

Without the "additionalEntires" option, the files are reported as "untouched" with 0% coverage which is correct but therefore not considered in the overall coverage calculation, e.g.

If adding all source files as "additionalEntries", the files are reported as "touched" with still 0% coverage (and red) and considered in the overall coverage calculation, providing a more accurate image, e.g.

The problem seems to be caused by cypress-io/code-coverage#207 and its related PRhttps://github.com/cypress-io/code-coverage/pull/208 .

See Readme of my updated working example for a (not so nice) workaround to add all source files as additional entries in Cypress' webpack preprocessor: https://github.com/toaditoad/code-coverage-mwe

[lmiller1990]

Before going too far into implementing this, is the only benefit here to make the files show as red and make the image nicer?

It seems unusual to include files that are not otherwise used in a code coverage report, since the goal of code coverage is to report coverage on files that are actually included in your bundle.

[toaditoad]

The issue is that only source files that are touched by a test (!) are considered in the report. A source file that is part of the bundle but for some reason not touched during a test (e.g. some helper function that is imported) is not considered.

As seen in the screenshots above, this messes up the overall coverage: Take this example: An application consists of source file Foo and Bar while Bar providing some helper function that is imported in Foo. I could write a test Foo.spec and generate a code coverage report showing 100% code coverage with Foo being green and Bar being not considered (like in the first screenshot the last four source files). In my opinion, this is misleading because not 100% of my source code is covered by tests but my tests cover 100% of all touched source files.
Only by adding Bar explicitly, the report says: Oh, your tests cover Foo but not Bar, so your coverage is only 50% (assuming both files have the same amount of code).

[bahmutov]

You probably would be better specifying which files to include in the code coverage rather than force-including them. See https://github.com/cypress-io/code-coverage#include-code

…

On Tue, Jul 13, 2021 at 1:55 AM toaditoad ***@***.***> wrote: The issue is that only source files that are touched by a test (!) are considered in the report. A source file that is part of the bundle but for some reason not touched during a test (e.g. some helper function that is imported) is not considered. As seen in the screenshots above, this messes up the overall coverage: Take this example: An application consists of source file Foo and Bar while Bar providing some helper function that is imported in Foo. I could write a test Foo.spec and generate a code coverage report showing 100% code coverage with Foo being green and Bar being not considered (like in the first screenshot the last four source files). In my opinion, this is misleading because not 100% of my source code is covered by tests but my tests cover 100% of all touched source files. Only by adding Bar explicitly, the report says: Oh, your tests cover Foo but not Bar, so your coverage is only 50% (assuming both files have the same amount of code). — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#17230 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAQ4BJQ5P74N4JSEUGVINX3TXPIMXANCNFSM476IBZVA> .

-- Dr. Gleb Bahmutov, PhD Schedule video chat / phone call / meeting with me via https://calendly.com/bahmutov ***@***.*** @bahmutov ***@***.***> https://glebbahmutov.com/ https://glebbahmutov.com/blog https://github.com/bahmutov

[toaditoad]

Thanks, @bahmutov. That was my initial thought, too. However, looking at your issue cypress-io/code-coverage#207 with the related PR cypress-io/code-coverage#208 , you state:

inserts missing files into .nyc_output/out.json file before generating reports (as placeholder objects, not real coverage)

Just adding them as placeholders does not make sense to me because the overall coverage results would be misleading (as explained in my previous comment).

[sjlutterbie]

I've also run into this issue and would like to see it resolved, as I think there's a valid use case with regards to component testing:

We have a package of existing React components, for which we're in the process of writing Cypress component tests. When we check our code coverage, it shows the untested component files, but doesn't include them in the overall coverage calculation. This means that our project is reported to have ~50% coverage, when in fact only a fraction of the components have tests associated with them.

[lmiller1990]

If anyone wants to take a stab, happy to provide feedback/guidance. I don't think I will be able to work on this in the immediate future, but sounds like it's a useful feature to maintain parity between the preprocessor and dev server plugins. You could probably look at the preprocessor implementation for inspiration.

[indatawetrust]

is there an up-to-date solution for this?

[lmiller1990]

This should not be too difficult to implement, but I'm wondering if it's not already possible via the huge amount of options webpack exposes?

We grab the your webpack by default, or you can specify additional configuration:

import { defineConfig } from 'cypress'
import webpackConfig from './webpack.config'

export default defineConfig({
  component: {
    devServer: {
      framework: 'react',
      bundler: 'webpack',
      // optionally pass in webpack config, or modify it
      webpackConfig,
    },
  },
})

I wonder if it's possible to do this in some fashion in user land? Likely with a webpack plugin, either custom or existing one - something like

plugins: [
  new AddAddidtionalEntriesPlugin({
    components: '**/*.jsx'
  })
]

That said, we can probably add an additionalEntries to our webpack-dev-server. One thing to keep in mind - this will slow down your runs, at least the initial compilation, since you will be effectively be bundling all your components.

[denieler]

We also stuck with this behavior of code coverage, the current code coverage statistic is misleading, because it doesn't include files, which are not touched by any tests

[lmiller1990]

Any interest in making a PR to add this feature? I think this should be possible without making a PR though, using one of the many existing webpack plugins, I think there should be something to add extra entry points to the dev server?

[denieler]

@lmiller1990 as I know Cypress started to work differently than before (at least for Cypress Component testing). You don't bundle the whole code to execute a single test in the browser, you use the test file as an entry point for webpack on each test.

Taking this into account additionalEntries approach might not work now or would cause horrible performance downsides. I believe the better approach would be to pre-process files before tests start and collect the number of lines, statements, branches, etc. before webpack. WDYT?

[denieler]

I've found full code coverage PR for Jest, here it is - https://github.com/facebook/jest/pull/1354/files

By analogy, I've written a script:

// collect-full-coverage.js

const path = require('path')
const fs = require('fs')
const babel = require('@babel/core')
const IstanbulInstrument = require('istanbul-lib-instrument')

/* 
  example: 
  [
    'cypress/**//*.{js,jsx,ts,tsx}',
    'coverage/**//*'
  ]
*/
const nycIgnorePatterns = require('./nycExcludeFiles.js')
const includePatterns = [
  '**/*.{js,jsx,ts,tsx}',
  // or other files to include
]

const coveragePath = path.join(__dirname, './coverage/cypress/coverage-final.json')

const isEmpty = obj => {
  return Object.keys(obj).length === 0
}

const generateEmptyCoverage = (source, filename) => {
  const instrumenter = IstanbulInstrument.createInstrumenter()
  const transpiledSource = babel.transformSync(source, {
    filename,
  })
  instrumenter.instrumentSync(transpiledSource.code, filename)
  return instrumenter.fileCoverage
}

const readCoverage = coveragePath => {
  return JSON.parse(fs.readFileSync(coveragePath, 'utf8'))
}

const saveCoverage = (coverageMap, coveragePath) => {
  fs.writeFileSync(coveragePath, JSON.stringify(coverageMap, null, 2))
}

const addToCoverage = (coverageMap, coverage) => {
  coverageMap[coverage.path] = coverage
}

const addUntestedFiles = async () => {
  const { globby } = await import('globby')

  const globbyPattern = [
    ...includePatterns,
    ...nycIgnorePatterns.map(pattern => `!${pattern}`)
  ]
  console.log('globbyPattern:', globbyPattern)
  const files = await globby(
    globbyPattern,
    {
      gitignore: true
    }
  )

  const coverageMap = readCoverage(coveragePath)

  files.forEach(filename => {
    const absoluteFilename = path.resolve(filename)

    if (coverageMap[absoluteFilename] && !isEmpty(coverageMap[absoluteFilename].statementMap)) {
      return
    }

    console.log('file:', absoluteFilename)

    try {
      const source = fs.readFileSync(absoluteFilename).toString()
      
      addToCoverage(
        coverageMap,
        generateEmptyCoverage(source, absoluteFilename)
      )
    } catch (e) {
      console.error(`
        Failed to collect coverage from ${absoluteFilename}
        ERROR: ${e}
        STACK: ${e.stack}
      `)
    }
  })

  saveCoverage(coverageMap, coveragePath)
}

addUntestedFiles()

You can execute it after Cypress tests are run and coverage is collected in coverage/cypress/coverage-final.json:

node ./collect-full-coverage.js

This script would adjust your final coverage file and add files, which have not been "touched" by tests processor.
I'm sorry, I don't have time now to prepare a proper PR to Cypress, but maybe for somebody, it would be a good start to prepare one 🙇‍♂️

[lmiller1990]

Thanks for posting ^, nicely done!

I am not sure on the best way to handle this - I agree we don't want to impact perf, but just having code coverage already impacts performance.

I can't prioritize adding this feature right now, I'm sorry - if anyone wants to try, I'd recommend proposing an approach/doing some experimentation before trying to make a PR. I think a solution like ^ in the form of a webpack plugin (if possible) would be even better.

[cypress-app-bot]

This issue has not had any activity in 180 days. Cypress evolves quickly and the reported behavior should be tested on the latest version of Cypress to verify the behavior is still occurring. It will be closed in 14 days if no updates are provided.

[cypress-app-bot]

This issue has been closed due to inactivity.

[peterhodges]

I've spent the best part of 2 days setting up code coverage on an NX workspace using Cypress component testing and have run into this issue too.

I came to the same conclusion before finding this thread. The "placeholder" coverage entries that cypress/coverage adds to nyc_output aren't sufficient to build a meaningful report.

Thanks @denieler for the script above. It'll save me some time!

It'd be great to productise this into cypress though.

[kevingorry]

Agreed, we would really benefit from this feature. @lmiller1990 any update ?

[CedricHg]

I don't understand why not taking untouched files into account at all is the default behaviour. In which scenario does that actually result in a useful statistic? At the very least maybe the documentation can be updated to better reflect this reality cause like others I've wasted a lot of time investigating this.