Skip to content

Commit

Permalink
Improve code quality by increasing code coverage: lib/source-map-from…
Browse files Browse the repository at this point in the history
…-file.js (bcoe#453)

refactor:  lib/source-map-from-file.js to improve code coverage
refactor: exposed source-map-from-file.js function to write test cases
test: Added two test case cases to cover error handling for improper formatting
  • Loading branch information
mcknasty committed Jan 20, 2024
1 parent bf3073b commit 8e093d8
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 48 deletions.
2 changes: 1 addition & 1 deletion lib/report.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ try {
const { readdirSync, readFileSync, statSync } = require('fs')
const { isAbsolute, resolve, extname } = require('path')
const { pathToFileURL, fileURLToPath } = require('url')
const getSourceMapFromFile = require('./source-map-from-file')
const { getSourceMapFromFile } = require('./source-map-from-file')
// TODO: switch back to @c88/v8-coverage once patch is landed.
const v8toIstanbul = require('v8-to-istanbul')
const util = require('util')
Expand Down
12 changes: 11 additions & 1 deletion lib/source-map-from-file.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,13 @@ function sourceMapFromDataUrl (url) {
const { 0: format, 1: data } = url.split(',')
const splitFormat = format.split(';')
const contentType = splitFormat[0]
// always evaluates to true for our use cases. Could possibly be false.
// Per https://github.com/bcoe/c8/pull/453#discussion_r1106306167
const base64 = splitFormat[splitFormat.length - 1] === 'base64'
if (contentType === 'application/json') {
// the data variable is never a json string in the test cases
// the upstream branch in node.js still has this logic
// https://github.com/nodejs/node/blob/v19.x/lib/internal/source_map/source_map_cache.js#L241
const decodedData = base64 ? Buffer.from(data, 'base64').toString('utf8') : data
try {
return JSON.parse(decodedData)
Expand All @@ -97,4 +102,9 @@ function sourceMapFromDataUrl (url) {
}
}

module.exports = getSourceMapFromFile
module.exports = {
getSourceMapFromFile,
dataFromUrl,
sourceMapFromFile,
sourceMapFromDataUrl
}
66 changes: 25 additions & 41 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
"dependencies": {
"@bcoe/v8-coverage": "^0.2.3",
"@istanbuljs/schema": "^0.1.3",
"chai-spies": "^1.0.0",
"find-up": "^5.0.0",
"foreground-child": "^3.1.1",
"istanbul-lib-coverage": "^3.2.0",
Expand Down
8 changes: 4 additions & 4 deletions test/integration.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ hey
---------------------------------------|---------|----------|---------|---------|------------------------
File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
---------------------------------------|---------|----------|---------|---------|------------------------
All files | 3.52 | 12.5 | 6.52 | 3.52 |
All files | 3.5 | 12.5 | 6.52 | 3.5 |
c8 | 0 | 0 | 0 | 0 |
index.js | 0 | 0 | 0 | 0 | 1
c8/bin | 0 | 0 | 0 | 0 |
Expand All @@ -168,7 +168,7 @@ All files | 3.52 | 12.5 | 6.52 | 3.52
c8/lib | 0 | 0 | 0 | 0 |
parse-args.js | 0 | 0 | 0 | 0 | 1-224
report.js | 0 | 0 | 0 | 0 | 1-402
source-map-from-file.js | 0 | 0 | 0 | 0 | 1-100
source-map-from-file.js | 0 | 0 | 0 | 0 | 1-110
c8/lib/commands | 0 | 0 | 0 | 0 |
check-coverage.js | 0 | 0 | 0 | 0 | 1-70
report.js | 0 | 0 | 0 | 0 | 1-43
Expand Down Expand Up @@ -521,7 +521,7 @@ hey
---------------------------------------|---------|----------|---------|---------|------------------------
File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
---------------------------------------|---------|----------|---------|---------|------------------------
All files | 3.52 | 12.5 | 6.52 | 3.52 |
All files | 3.5 | 12.5 | 6.52 | 3.5 |
c8 | 0 | 0 | 0 | 0 |
index.js | 0 | 0 | 0 | 0 | 1
c8/bin | 0 | 0 | 0 | 0 |
Expand All @@ -533,7 +533,7 @@ All files | 3.52 | 12.5 | 6.52 | 3.52
c8/lib | 0 | 0 | 0 | 0 |
parse-args.js | 0 | 0 | 0 | 0 | 1-224
report.js | 0 | 0 | 0 | 0 | 1-402
source-map-from-file.js | 0 | 0 | 0 | 0 | 1-100
source-map-from-file.js | 0 | 0 | 0 | 0 | 1-110
c8/lib/commands | 0 | 0 | 0 | 0 |
check-coverage.js | 0 | 0 | 0 | 0 | 1-70
report.js | 0 | 0 | 0 | 0 | 1-43
Expand Down
40 changes: 39 additions & 1 deletion test/source-map-from-file.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
/* global describe, it */
const getSourceMapFromFile = require('../lib/source-map-from-file')
const { getSourceMapFromFile, sourceMapFromDataUrl } = require('../lib/source-map-from-file')
const assert = require('assert')
const { readFileSync } = require('fs')
const chia = require('chai')
const spies = require('chai-spies')

chia.use(spies)

const { expect } = chia

describe('source-map-from-file', () => {
it('should parse source maps from compiled targets', () => {
const sourceMap = getSourceMapFromFile('./test/fixtures/all/ts-compiled/main.js')
Expand All @@ -16,4 +23,35 @@ describe('source-map-from-file', () => {
const sourceMap = getSourceMapFromFile('./test/fixtures/source-maps/inline.js')
assert.strictEqual(sourceMap.version, 3)
})

it('should throw error if source map isn\'t JSON format', () => {
const spy = chia.spy
spy.on(JSON, 'parse', (content) => {
throw new Error('No actual error. Just testing code coverage. ')
})
try {
getSourceMapFromFile('./test/fixtures/all/ts-compiled/main.js')
} finally {
expect(JSON.parse).to.have.been.called()
spy.restore()
}

spy.on(JSON, 'parse', (content) => {
throw new Error('No actual error. Just testing code coverage. ')
})
try {
const mockUrl = 'application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJzb...'
const sourceMap = sourceMapFromDataUrl(mockUrl)
expect(sourceMap).to.equal(null)
} finally {
expect(JSON.parse).to.have.been.called()
spy.restore()
}
})

it('should throw error if url doesn\'t have a json content type', () => {
const mockUrl = 'application/text;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJzb...'
const sourceMap = sourceMapFromDataUrl(mockUrl)
expect(sourceMap).to.equal(null)
})
})

0 comments on commit 8e093d8

Please sign in to comment.