Skip to content

Commit a280451

Browse files
AriPerkkiosheremet-va
andauthoredFeb 20, 2024··
fix(coverage): ignore generated TS decorators (#5206)
Co-authored-by: Vladimir <sleuths.slews0s@icloud.com>
1 parent 5376d5b commit a280451

File tree

13 files changed

+1059
-66
lines changed

13 files changed

+1059
-66
lines changed
 

‎packages/coverage-istanbul/src/provider.ts

+3
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,9 @@ export class IstanbulCoverageProvider extends BaseCoverageProvider implements Co
114114
const sourceMap = pluginCtx.getCombinedSourcemap()
115115
sourceMap.sources = sourceMap.sources.map(removeQueryParameters)
116116

117+
// Exclude SWC's decorators that are left in source maps
118+
sourceCode = sourceCode.replaceAll('_ts_decorate', '/* istanbul ignore next */_ts_decorate')
119+
117120
const code = this.instrumenter.instrumentSync(sourceCode, id, sourceMap as any)
118121
const map = this.instrumenter.lastSourceMap() as any
119122

‎packages/coverage-v8/src/provider.ts

+14-10
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ const WRAPPER_LENGTH = 185
5151

5252
// Note that this needs to match the line ending as well
5353
const VITE_EXPORTS_LINE_PATTERN = /Object\.defineProperty\(__vite_ssr_exports__.*\n/g
54+
const DECORATOR_METADATA_PATTERN = /_ts_metadata\("design:paramtypes"(\s|.)+?]\),/g
5455
const DEFAULT_PROJECT = Symbol.for('default-project')
5556

5657
const debug = createDebug('vitest:coverage')
@@ -312,7 +313,7 @@ export class V8CoverageProvider extends BaseCoverageProvider implements Coverage
312313
originalSource: sourcesContent,
313314
source: code || sourcesContent,
314315
sourceMap: {
315-
sourcemap: removeViteHelpersFromSourceMaps(code, {
316+
sourcemap: excludeGeneratedCode(code, {
316317
...map,
317318
version: 3,
318319
sources: [url],
@@ -363,21 +364,24 @@ async function transformCoverage(coverageMap: CoverageMap) {
363364
/**
364365
* Remove generated code from the source maps:
365366
* - Vite's export helpers: e.g. `Object.defineProperty(__vite_ssr_exports__, "sum", { enumerable: true, configurable: true, get(){ return sum }});`
367+
* - SWC's decorator metadata: e.g. `_ts_metadata("design:paramtypes", [\ntypeof Request === "undefined" ? Object : Request\n]),`
366368
*/
367-
function removeViteHelpersFromSourceMaps(source: string | undefined, map: EncodedSourceMap) {
368-
if (!source || !source.match(VITE_EXPORTS_LINE_PATTERN))
369+
function excludeGeneratedCode(source: string | undefined, map: EncodedSourceMap) {
370+
if (!source)
369371
return map
370372

371-
const sourceWithoutHelpers = new MagicString(source)
372-
sourceWithoutHelpers.replaceAll(VITE_EXPORTS_LINE_PATTERN, '\n')
373+
if (!source.match(VITE_EXPORTS_LINE_PATTERN) && !source.match(DECORATOR_METADATA_PATTERN))
374+
return map
375+
376+
const trimmed = new MagicString(source)
377+
trimmed.replaceAll(VITE_EXPORTS_LINE_PATTERN, '\n')
378+
trimmed.replaceAll(DECORATOR_METADATA_PATTERN, match => '\n'.repeat(match.split('\n').length - 1))
373379

374-
const mapWithoutHelpers = sourceWithoutHelpers.generateMap({
375-
hires: 'boundary',
376-
})
380+
const trimmedMap = trimmed.generateMap({ hires: 'boundary' })
377381

378-
// A merged source map where the first one excludes helpers
382+
// A merged source map where the first one excludes generated parts
379383
const combinedMap = remapping(
380-
[{ ...mapWithoutHelpers, version: 3 }, map],
384+
[{ ...trimmedMap, version: 3 }, map],
381385
() => null,
382386
)
383387

‎pnpm-lock.yaml

+172
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/coverage-test/coverage-report-tests/__snapshots__/custom.report.test.ts.snap

+1
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ exports[`custom json report 1`] = `
2020
"<process-cwd>/src/Defined.vue",
2121
"<process-cwd>/src/Hello.vue",
2222
"<process-cwd>/src/another-setup.ts",
23+
"<process-cwd>/src/decorators.ts",
2324
"<process-cwd>/src/dynamic-file-esm.ignore.js",
2425
"<process-cwd>/src/dynamic-files.ts",
2526
"<process-cwd>/src/function-count.ts",

‎test/coverage-test/coverage-report-tests/__snapshots__/istanbul.report.test.ts.snap

+209
Original file line numberDiff line numberDiff line change
@@ -530,6 +530,215 @@ exports[`istanbul json report 1`] = `
530530
},
531531
},
532532
},
533+
"<process-cwd>/src/decorators.ts": {
534+
"b": {
535+
"0": [
536+
1,
537+
0,
538+
],
539+
"1": [
540+
0,
541+
1,
542+
],
543+
},
544+
"branchMap": {
545+
"0": {
546+
"loc": {
547+
"end": {
548+
"column": null,
549+
"line": 8,
550+
},
551+
"start": {
552+
"column": 4,
553+
"line": 5,
554+
},
555+
},
556+
"locations": [
557+
{
558+
"end": {
559+
"column": null,
560+
"line": 8,
561+
},
562+
"start": {
563+
"column": 4,
564+
"line": 5,
565+
},
566+
},
567+
{
568+
"end": {
569+
"column": null,
570+
"line": 8,
571+
},
572+
"start": {
573+
"column": 4,
574+
"line": 5,
575+
},
576+
},
577+
],
578+
"type": "if",
579+
},
580+
"1": {
581+
"loc": {
582+
"end": {
583+
"column": null,
584+
"line": 13,
585+
},
586+
"start": {
587+
"column": 4,
588+
"line": 10,
589+
},
590+
},
591+
"locations": [
592+
{
593+
"end": {
594+
"column": null,
595+
"line": 13,
596+
},
597+
"start": {
598+
"column": 4,
599+
"line": 10,
600+
},
601+
},
602+
{
603+
"end": {
604+
"column": null,
605+
"line": 13,
606+
},
607+
"start": {
608+
"column": 4,
609+
"line": 10,
610+
},
611+
},
612+
],
613+
"type": "if",
614+
},
615+
},
616+
"f": {
617+
"0": 1,
618+
"1": 1,
619+
"2": 1,
620+
},
621+
"fnMap": {
622+
"0": {
623+
"decl": {
624+
"end": {
625+
"column": 9,
626+
"line": 4,
627+
},
628+
"start": {
629+
"column": 2,
630+
"line": 4,
631+
},
632+
},
633+
"loc": {
634+
"end": {
635+
"column": null,
636+
"line": 14,
637+
},
638+
"start": {
639+
"column": 46,
640+
"line": 4,
641+
},
642+
},
643+
"name": "(anonymous_4)",
644+
},
645+
"1": {
646+
"decl": {
647+
"end": {
648+
"column": null,
649+
"line": 17,
650+
},
651+
"start": {
652+
"column": 9,
653+
"line": 17,
654+
},
655+
},
656+
"loc": {
657+
"end": {
658+
"column": null,
659+
"line": 21,
660+
},
661+
"start": {
662+
"column": 25,
663+
"line": 20,
664+
},
665+
},
666+
"name": "SomeDecorator",
667+
},
668+
"2": {
669+
"decl": {
670+
"end": {
671+
"column": 17,
672+
"line": 25,
673+
},
674+
"start": {
675+
"column": 9,
676+
"line": 25,
677+
},
678+
},
679+
"loc": {
680+
"end": {
681+
"column": null,
682+
"line": 27,
683+
},
684+
"start": {
685+
"column": 33,
686+
"line": 25,
687+
},
688+
},
689+
"name": "noop",
690+
},
691+
},
692+
"path": "<process-cwd>/src/decorators.ts",
693+
"s": {
694+
"0": 1,
695+
"1": 1,
696+
"2": 1,
697+
"3": 0,
698+
},
699+
"statementMap": {
700+
"0": {
701+
"end": {
702+
"column": null,
703+
"line": 8,
704+
},
705+
"start": {
706+
"column": 4,
707+
"line": 5,
708+
},
709+
},
710+
"1": {
711+
"end": {
712+
"column": null,
713+
"line": 7,
714+
},
715+
"start": {
716+
"column": 6,
717+
"line": 7,
718+
},
719+
},
720+
"2": {
721+
"end": {
722+
"column": null,
723+
"line": 13,
724+
},
725+
"start": {
726+
"column": 4,
727+
"line": 10,
728+
},
729+
},
730+
"3": {
731+
"end": {
732+
"column": null,
733+
"line": 12,
734+
},
735+
"start": {
736+
"column": 6,
737+
"line": 12,
738+
},
739+
},
740+
},
741+
},
533742
"<process-cwd>/src/dynamic-files.ts": {
534743
"b": {
535744
"0": [

‎test/coverage-test/coverage-report-tests/__snapshots__/v8.report.test.ts.snap

+504
Large diffs are not rendered by default.

‎test/coverage-test/coverage-report-tests/generic.report.test.ts

+24
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,30 @@ test('multi environment coverage is merged correctly', async () => {
185185
expect(lineCoverage[30]).toBe(2)
186186
})
187187

188+
test('decorators generated metadata is ignored', async () => {
189+
const coverageJson = await readCoverageJson()
190+
const coverageMap = libCoverage.createCoverageMap(coverageJson as any)
191+
const fileCoverage = coverageMap.fileCoverageFor('<process-cwd>/src/decorators.ts')
192+
const lineCoverage = fileCoverage.getLineCoverage()
193+
const branchCoverage = fileCoverage.getBranchCoverageByLine()
194+
195+
// Decorator should not be uncovered - on V8 this is marked as covered, on Istanbul it's excluded from report
196+
if (process.env.COVERAGE_PROVIDER === 'v8') {
197+
expect(lineCoverage['4']).toBe(1)
198+
expect(branchCoverage['4'].coverage).toBe(100)
199+
}
200+
else {
201+
expect(lineCoverage['4']).toBeUndefined()
202+
expect(branchCoverage['4']).toBeUndefined()
203+
}
204+
205+
// Covered branch should be marked correctly
206+
expect(lineCoverage['7']).toBe(1)
207+
208+
// Uncovered branch should be marked correctly
209+
expect(lineCoverage['12']).toBe(0)
210+
})
211+
188212
test('temporary files are removed after test', async () => {
189213
const coveragePath = resolve('./coverage')
190214
const files = fs.readdirSync(coveragePath)

‎test/coverage-test/package.json

+1
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
"istanbul-lib-coverage": "^3.2.0",
2525
"istanbul-lib-report": "^3.0.1",
2626
"magicast": "^0.3.3",
27+
"unplugin-swc": "^1.4.4",
2728
"vite": "latest",
2829
"vitest": "workspace:*",
2930
"vue": "latest",

‎test/coverage-test/src/decorators.ts

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// eslint-disable-next-line ts/ban-ts-comment -- so that typecheck doesn't include it
2+
// @ts-nocheck
3+
export class DecoratorsTester {
4+
method(@SomeDecorator parameter: Something) {
5+
if (parameter) {
6+
// Covered line
7+
noop(parameter)
8+
}
9+
10+
if (parameter === 'uncovered') {
11+
// Uncovered line
12+
noop(parameter)
13+
}
14+
}
15+
}
16+
17+
function SomeDecorator(
18+
_target: Object,
19+
_propertyKey: string | symbol,
20+
_parameterIndex: number,
21+
) {}
22+
23+
type Something = unknown
24+
25+
function noop(..._args: unknown[]) {
26+
27+
}

‎test/coverage-test/test/coverage.test.ts

+5
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { implicitElse } from '../src/implicitElse'
77
import { useImportEnv } from '../src/importEnv'
88
import { second } from '../src/function-count'
99
import MultiSuite from '../src/multi-suite'
10+
import { DecoratorsTester } from '../src/decorators'
1011

1112
// @ts-expect-error -- untyped virtual file provided by custom plugin
1213
import virtualFile2 from '\0vitest-custom-virtual-file-2'
@@ -71,3 +72,7 @@ test('virtual file imports', () => {
7172
expect(virtualFile1).toBe('This file should be excluded from coverage report #1')
7273
expect(virtualFile2).toBe('This file should be excluded from coverage report #2')
7374
})
75+
76+
test('decorators', () => {
77+
new DecoratorsTester().method('cover line')
78+
})

‎test/coverage-test/testing.mjs

+2-2
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ const isBrowser = process.argv.includes('--browser')
88
const isCI = process.env.GITHUB_ACTIONS
99
process.env.COVERAGE_PROVIDER = provider
1010

11-
// TODO: Fix flakiness and enable on CI -- browser picks test files that don't exist and fails, some tests fail because of the multi environment mismatch
12-
if (isCI)
11+
// TODO: Fix flakiness and enable on CI -- browser picks test files that don't exist and fails, issue #5165
12+
if (isCI && isBrowser)
1313
process.exit(0)
1414

1515
const poolConfigs = [

‎test/coverage-test/tsconfig.json

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"extends": "../../tsconfig.base.json",
3+
"compilerOptions": {
4+
"experimentalDecorators": true
5+
}
6+
}

‎test/coverage-test/vitest.config.ts

+91-54
Original file line numberDiff line numberDiff line change
@@ -1,67 +1,19 @@
11
import { resolve } from 'pathe'
2+
import swc from 'unplugin-swc'
23
import { defineConfig } from 'vitest/config'
34
import vue from '@vitejs/plugin-vue'
45
import MagicString from 'magic-string'
56
import remapping from '@ampproject/remapping'
7+
import type { Plugin } from 'vite'
68

79
const provider = process.argv[1 + process.argv.indexOf('--provider')]
810

9-
export default defineConfig(() => ({
11+
export default defineConfig(_ => ({
1012
plugins: [
1113
vue(),
12-
/*
13-
* Transforms `multi-environment.ts` differently based on test environment (JSDOM/Node)
14-
* so that there are multiple different source maps for a single file.
15-
* This causes a case where coverage report is incorrect if sourcemaps are not picked based on transform mode.
16-
*/
17-
{
18-
name: 'vitest-custom-multi-transform',
19-
enforce: 'pre',
20-
transform(code, id, options) {
21-
if (id.includes('src/multi-environment')) {
22-
const ssr = options?.ssr || false
23-
const transforMode = `transformMode is ${ssr ? 'ssr' : 'csr'}`
24-
const padding = '\n*****'.repeat(ssr ? 0 : 15)
25-
26-
const transformed = new MagicString(code)
27-
transformed.replace('\'default-padding\'', `\`${transforMode} ${padding}\``)
28-
29-
const map = remapping(
30-
[transformed.generateMap({ hires: true }), this.getCombinedSourcemap() as any],
31-
() => null,
32-
) as any
33-
34-
return { code: transformed.toString(), map }
35-
}
36-
},
37-
},
38-
{
39-
// Simulates Vite's virtual files: https://vitejs.dev/guide/api-plugin.html#virtual-modules-convention
40-
name: 'vitest-custom-virtual-files',
41-
resolveId(id) {
42-
if (id === 'virtual:vitest-custom-virtual-file-1')
43-
return 'src/virtual:vitest-custom-virtual-file-1.ts'
44-
45-
if (id === '\0vitest-custom-virtual-file-2')
46-
return 'src/\0vitest-custom-virtual-file-2.ts'
47-
},
48-
load(id) {
49-
if (id === 'src/virtual:vitest-custom-virtual-file-1.ts') {
50-
return `
51-
const virtualFile = "This file should be excluded from coverage report #1"
52-
export default virtualFile;
53-
`
54-
}
55-
56-
// Vitest browser resolves this as "\x00", Node as "__x00__"
57-
if (id === 'src/__x00__vitest-custom-virtual-file-2.ts' || id === 'src/\x00vitest-custom-virtual-file-2.ts') {
58-
return `
59-
const virtualFile = "This file should be excluded from coverage report #2"
60-
export default virtualFile;
61-
`
62-
}
63-
},
64-
},
14+
MultiTransformPlugin(),
15+
VirtualFilesPlugin(),
16+
DecoratorsPlugin(),
6517
],
6618
define: {
6719
MY_CONSTANT: '"my constant"',
@@ -105,3 +57,88 @@ export default defineConfig(() => ({
10557
],
10658
},
10759
}))
60+
61+
/*
62+
* Transforms `multi-environment.ts` differently based on test environment (JSDOM/Node)
63+
* so that there are multiple different source maps for a single file.
64+
* This causes a case where coverage report is incorrect if sourcemaps are not picked based on transform mode.
65+
*/
66+
function MultiTransformPlugin(): Plugin {
67+
return {
68+
name: 'vitest-custom-multi-transform',
69+
enforce: 'pre',
70+
transform(code, id, options) {
71+
if (id.includes('src/multi-environment')) {
72+
const ssr = options?.ssr || false
73+
const transforMode = `transformMode is ${ssr ? 'ssr' : 'csr'}`
74+
const padding = '\n*****'.repeat(ssr ? 0 : 15)
75+
76+
const transformed = new MagicString(code)
77+
transformed.replace('\'default-padding\'', `\`${transforMode} ${padding}\``)
78+
79+
const map = remapping(
80+
[transformed.generateMap({ hires: true }), this.getCombinedSourcemap() as any],
81+
() => null,
82+
) as any
83+
84+
return { code: transformed.toString(), map }
85+
}
86+
},
87+
}
88+
}
89+
90+
// Simulates Vite's virtual files: https://vitejs.dev/guide/api-plugin.html#virtual-modules-convention
91+
function VirtualFilesPlugin(): Plugin {
92+
return {
93+
name: 'vitest-custom-virtual-files',
94+
resolveId(id) {
95+
if (id === 'virtual:vitest-custom-virtual-file-1')
96+
return 'src/virtual:vitest-custom-virtual-file-1.ts'
97+
98+
if (id === '\0vitest-custom-virtual-file-2')
99+
return 'src/\0vitest-custom-virtual-file-2.ts'
100+
},
101+
load(id) {
102+
if (id === 'src/virtual:vitest-custom-virtual-file-1.ts') {
103+
return `
104+
const virtualFile = "This file should be excluded from coverage report #1"
105+
export default virtualFile;
106+
`
107+
}
108+
109+
// Vitest browser resolves this as "\x00", Node as "__x00__"
110+
if (id === 'src/__x00__vitest-custom-virtual-file-2.ts' || id === 'src/\x00vitest-custom-virtual-file-2.ts') {
111+
return `
112+
const virtualFile = "This file should be excluded from coverage report #2"
113+
export default virtualFile;
114+
`
115+
}
116+
},
117+
}
118+
}
119+
120+
function DecoratorsPlugin(): Plugin {
121+
const plugin = swc.vite({
122+
jsc: {
123+
target: 'esnext',
124+
parser: {
125+
syntax: 'typescript',
126+
decorators: true,
127+
},
128+
transform: {
129+
legacyDecorator: true,
130+
decoratorMetadata: true,
131+
},
132+
},
133+
})
134+
135+
return {
136+
name: 'custom-swc-decorator',
137+
enforce: 'pre',
138+
transform(code, id, options) {
139+
if (id.endsWith('decorators.ts'))
140+
// @ts-expect-error -- Ignore complex type
141+
return plugin.transform(code, id, options)
142+
},
143+
}
144+
}

0 commit comments

Comments
 (0)
Please sign in to comment.