test-runner icon indicating copy to clipboard operation
test-runner copied to clipboard

Merging test coverage with other reports generated by coverageProvider v8 result in wrong coverage

Open derweili opened this issue 2 years ago • 9 comments

Describe the bug

I setup coverage as described in the docs. https://storybook.js.org/addons/@storybook/test-runner#setting-up-code-coverage:~:text=published%20Storybook%20instead.-,Setting%20up%20code%20coverage,-The%20test%20runner I added the coverage addon, run yarn test-storybook --coverage. Now I want to merge it with my other jest coverage.

So I setup the described coverage-report script as described in the docs: https://storybook.js.org/addons/@storybook/test-runner#setting-up-code-coverage:~:text=3%20%2D%20Merging%20code%20coverage%20with%20coverage%20from%20other%20tools

My scripts look like this:

    "test:jest": "jest",
    "coverage": "yarn run coverage:storybook && yarn run coverage:unit && yarn run coverage:report",
    "coverage:unit": "yarn run test -- --watchAll=false --coverage",
    "coverage:storybook": "test-storybook --config-dir build/storybook --coverage",
    "coverage:report": "cp coverage/storybook/coverage-storybook.json coverage/coverage-storybook.json && nyc report --reporter=text -t coverage --report-dir coverage/merged",

When I run test-storybook and jest separately, everything looks good. Wenn both are merged the results look wrong.

Here is an example of my Navigation Component.

Result from the unit test:

 src/Navigation                |   47.22 |    23.53 |       0 |   59.65 |                                                                                            
  Navigation.tsx               |      50 |        0 |       0 |   58.33 | 16-20                                                                                      
  NavigationFooterLink.tsx     |     100 |      100 |     100 |     100 |                                                                                            
  NavigationHeaderLink.tsx     |   22.22 |    33.33 |       0 |   33.33 | 11-19                                                                                      
  NavigationItem.tsx           |   63.64 |        0 |       0 |   77.78 | 12-13                                                                                      
  NavigationItemRoot.ts        |   28.57 |       50 |       0 |      40 | 10-14                                                                                      
  NavigationRoot.ts            |   21.05 |     12.5 |       0 |   30.77 | 15-40                                                                                      
  index.ts                     |     100 |      100 |       0 |     100 |    

Result from test-storybook:

src/Navigation                |   85.71 |    66.67 |      75 |   85.71 |                                                                                                                    
  Navigation.tsx               |     100 |      100 |     100 |     100 |                                                                                                                    
  NavigationFooterLink.tsx     |     100 |      100 |     100 |     100 |                                                                                                                    
  NavigationHeaderLink.tsx     |      25 |        0 |       0 |      25 | 11-19                                                                                                              
  NavigationItem.tsx           |     100 |      100 |     100 |     100 |                                                                                                                    
  NavigationItemRoot.ts        |     100 |       50 |     100 |     100 | 14                                                                                                                 
  NavigationRoot.ts            |     100 |    78.57 |     100 |     100 | 19,25,40                                                                                                           
  index.ts                     |     100 |      100 |     100 |     100 | 

Result after merge:

src/Navigation                |   48.61 |    61.76 |   64.29 |    61.4 |                                                                                                             
  Navigation.tsx               |      50 |       50 |     100 |   58.33 | 16-20                                                                                                       
  NavigationFooterLink.tsx     |     100 |      100 |     100 |     100 |                                                                                                             
  NavigationHeaderLink.tsx     |   22.22 |    33.33 |       0 |   33.33 | 11-19                                                                                                       
  NavigationItem.tsx           |   63.64 |      100 |     100 |   77.78 | 12-13                                                                                                       
  NavigationItemRoot.ts        |   28.57 |       50 |     100 |      40 | 10-14                                                                                                       
  NavigationRoot.ts            |   26.32 |    68.75 |     100 |   38.46 | 18-40                                                                                                       
  index.ts                     |     100 |      100 |       0 |     100 |

Why is the result after merge lower than the result after test-storybook? Navigation.tsx (line 2) has a test coverage of 100% in test-storybook so no other tests are required from unit test. So I would expect 100% coverage after merge. But the coverage after merge is 50% like in the unit test.

The Content of the mentioned `Navigation.tsx` looks like this (click to show code)
import React from 'react'
import { Size } from '../design-system/Styleguide'
import NavigationRoot from './NavigationRoot'

type Props = {
  orientation?: 'horizontal' | 'vertical'
  spacing?: Size | false
  size?: Size | false
  children: React.ReactNode
}

const defaultSpacing = Size.Medium
const defaultSize = Size.Medium

const Navigation = ({
  spacing = defaultSpacing,
  size = defaultSize,
  orientation,
  children,
}: Props) => (
  <NavigationRoot
    gridSpacing={spacing}
    gridSize={size}
    orientation={orientation}
  >
    {children}
  </NavigationRoot>
)

export default Navigation

Environment

  • OS: macOS 12.2.1
  • Node.js version: 16.13.2
  • YARN version: 1.22.18
  • Browser (if applicable): Chrome
  • Browser version (if applicable): https://www.whatsmybrowser.org/b/T2EFP
  • Device (if applicable): MacBook 14" 2021 M1

derweili avatar Jul 15 '22 10:07 derweili

I ran into a similar problem. There's not enough information here to tell if it's the same, but do you see a different number of line counts between the two coverage reports?

I found that my problem was when I used coverageProvider: 'v8' as recommended in the next.js docs import and export lines were counted as coverable pieces of code (which makes sense if you think about how v8 and modules work). Storybook's test runner is using babel for the coverageProvider, and so the import/export lines do not count.

So the coverage report by storybook was reporting 100% covered and my jest tests were reporting 0%, and when merged I got about 50% because the storybook coverage report didn't "count" the import/export lines.

The fix I did was to change Next.js to use coverageProvider: 'babel' and then the reports lined up and merged as expected.

VickyKoblinski avatar Jul 17 '22 06:07 VickyKoblinski

@VickyKoblinski thank you for this information. I actually don't know anything about how coverage works, so I will check this coverage provider setting.

I'm not using nextjs but "pure" jest, but I will check what I can do here.

derweili avatar Jul 17 '22 11:07 derweili

@VickyKoblinski what do you mean with "but do you see a different number of line counts between the two coverage reports"

Where can I see this?

derweili avatar Jul 17 '22 12:07 derweili

@VickyKoblinski it looks like you are right about the reason for my problem. The Jest unit tests has a line coverage of 4/13 while storybook test-runner has 5/5

So jest recognizes 13 lines while test runner only recognizes 5. The same happens to branches (jest 16, test runner 14) and statements (jest 19, test-runner 5).

That means I have to configure it somehow so that both recognize the same number of total lines to cover, either 13 or 5.

derweili avatar Jul 17 '22 15:07 derweili

So I tried to investigate my problem further. Looks like @VickyKoblinski is right with her assumption, the storybook test-runner and jest count the total numbers of lines (that need must be covered) differently

Unfortunately the coverageProvider: 'babel' setting did not work out for me. I was already using the default babel provider. But even with this default provider, storybook test-runner counts less lines than jest. For example import lines (screenshot lines 1-3) and lines that include object destructuring (e.g. screenshot lines 14, 18, 21) within function props of tagged template literals, are not counted as lines that need to be covered.

When enabling the mentioned v8 `coverageProvider?, jest also counts empty lines.

Here are screenshots of my results.:

Storybook Test runner: coverage-storybook

Jest:

coverage-jest-babel

How can I configure storybook test runner to use the same settings as default jest? Why does it behave differently?

derweili avatar Jul 17 '22 18:07 derweili

Hey @derweili thanks for explaining all of that and giving so much context. Would you mind setting up a small repro so I can work on top of? I'll need to figure some stuff out and that would be pretty helpful!

yannbf avatar Jul 17 '22 20:07 yannbf

@yannbf thank you for your help

Yes I was able to setup a project. unfortunately its not that small. Its a fork of my real project. https://github.com/derweili/storybook-test-runner-coverage-demo

You need to install dependencies yarn. The project runs with node^16.13

You can start storybook with yarn start

While storybook runs, you can then run yarn coverage This starts the mentioned unit and test-runner tests. The coverage reports are then saved to ./coverage/... It also creates a merged report

After generating the coverage reports you can open following files to see the reports from the screenshots above:

./coverage/unit/lcov-report/src/Navigation/NavigationRoot.ts.html # The Unit test result
./coverage/storybook/lcov-report/src/Navigation/NavigationRoot.ts.html # The test-runner test result

Again thank you for your help

derweili avatar Jul 17 '22 21:07 derweili

@derweili Hi, did you find soultion to the problem at the end?

VladimirBlagojevic avatar Feb 08 '24 11:02 VladimirBlagojevic

The root cause it can't merge is that the formats of the original coverage data are different.

  • 1, Stroybook: Istanbul coverage data instrumented with @storybook/addon-coverage
  • 2, Jest + coverageProvider: 'babel': Istanbul coverage data instrumented with babel-plugin-istanbul
  • 3, Jest + coverageProvider: 'v8': Istanbul coverage data converted with v8-to-istanbul from V8 coverage data

Options

  • Merging 1 with 2: It might succeed, but problems may still exist, because different tools babel and vite have been used, and different versions of istanbul-lib-instrument
"dependencies": {
    "@storybook/addon-coverage": "^1.0.1",
    "@storybook/test-runner": "^0.17.0"
  }
+-- @storybook/[email protected]
| +-- @jsdevtools/[email protected]
| | `-- [email protected]
| +-- [email protected]
| `-- [email protected]
|   `-- [email protected]
`-- @storybook/[email protected]
  +-- [email protected]
  | `-- [email protected]
  |   `-- [email protected]
  +-- [email protected]
  | `-- @jest/[email protected]
  |   `-- [email protected]
  |     `-- [email protected]
  `-- [email protected]
    `-- @jest/[email protected]
      `-- @jest/[email protected]
        `-- [email protected] deduped
  • Merging 1 with 3: It won't succeed in most cases, because we cannot perfectly convert v8 to Istanbul format with v8-to-istanbul, see issue

Possible solution: try using the native v8 coverage data source

cenfun avatar Mar 13 '24 04:03 cenfun