Skip to content

Commit 2026d4a

Browse files
antongolubmidzelisMin Idzelis
authored
refactor: enhance stack from (google#752)
* Fix stack extraction when running under bun * move stack extractor to function, add tests * perf: simplify stack extract * chore: linting * test: add bun smoke test * chore: linting --------- Co-authored-by: Min Idzelis <min123@gmail.com> Co-authored-by: Min Idzelis <midzelis@salesforce.com>
1 parent b02fd52 commit 2026d4a

File tree

7 files changed

+113
-8
lines changed

7 files changed

+113
-8
lines changed

.github/workflows/test.yml

+19
Original file line numberDiff line numberDiff line change
@@ -37,3 +37,22 @@ jobs:
3737
timeout-minutes: 1
3838
env:
3939
FORCE_COLOR: 3
40+
41+
bun:
42+
runs-on: ubuntu-latest
43+
44+
steps:
45+
- uses: actions/checkout@v3
46+
- name: Use Node.js 20.x
47+
uses: actions/setup-node@v3
48+
with:
49+
node-version: 20.x
50+
- name: Setup Bun
51+
uses: antongolub/action-setup-bun@v1
52+
53+
- run: npm ci
54+
- run: npm run build
55+
- run: bun test ./test/bun.test.js
56+
timeout-minutes: 1
57+
env:
58+
FORCE_COLOR: 3

src/core.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import {
3232
errnoMessage,
3333
exitCodeInfo,
3434
formatCmd,
35+
getCallerLocation,
3536
noop,
3637
parseDuration,
3738
quote,
@@ -128,7 +129,7 @@ export const $: Shell & Options = new Proxy<Shell & Options>(
128129
})
129130
}
130131
}
131-
const from = new Error().stack!.split(/^\s*at\s/m)[2].trim()
132+
const from = getCallerLocation()
132133
if (pieces.some((p) => p == undefined)) {
133134
throw new Error(`Malformed command at ${from}`)
134135
}

src/util.ts

+13
Original file line numberDiff line numberDiff line change
@@ -355,3 +355,16 @@ const reservedWords = [
355355
'done',
356356
'in',
357357
]
358+
359+
export function getCallerLocation(err = new Error()) {
360+
return getCallerLocationFromString(err.stack)
361+
}
362+
363+
export function getCallerLocationFromString(stackString = 'unknown') {
364+
return (
365+
stackString
366+
.split(/^\s*(at\s)?/m)
367+
.filter((s) => s?.includes(':'))[2]
368+
?.trim() || stackString
369+
)
370+
}

test/bun.test.js

+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// Copyright 2024 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// https://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
import assert from 'node:assert'
16+
import { test, describe } from 'bun:test'
17+
import '../build/globals.js'
18+
19+
describe('bun', () => {
20+
test('smoke test', async () => {
21+
const p = await $`echo foo`
22+
assert.match(p.stdout, /foo/)
23+
})
24+
25+
test('captures err stack', async () => {
26+
const p = await $({ nothrow: true })`echo foo; exit 3`
27+
assert.match(p.message, /exit code: 3/)
28+
})
29+
})

test/cli.test.js

+1-6
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
// limitations under the License.
1414

1515
import assert from 'node:assert'
16-
import { test, describe, before, beforeEach } from 'node:test'
16+
import { test, describe, beforeEach } from 'node:test'
1717
import '../build/globals.js'
1818

1919
describe('cli', () => {
@@ -70,11 +70,6 @@ describe('cli', () => {
7070
assert.match(out.stdout, /verbose is false/)
7171
})
7272

73-
test('supports `--experimental` flag', async () => {
74-
let out = await $`echo 'echo("test")' | node build/cli.js --experimental`
75-
assert.match(out.stdout, /test/)
76-
})
77-
7873
test('supports `--quiet` flag', async () => {
7974
let p = await $`node build/cli.js --quiet test/fixtures/markdown.md`
8075
assert.ok(!p.stderr.includes('ignore'), 'ignore was printed')

test/util.test.js

+48
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {
2424
quote,
2525
quotePowerShell,
2626
randomId,
27+
getCallerLocationFromString,
2728
} from '../build/util.js'
2829

2930
describe('util', () => {
@@ -92,3 +93,50 @@ describe('util', () => {
9293
)
9394
})
9495
})
96+
97+
test('getCallerLocation: empty', () => {
98+
assert.equal(getCallerLocationFromString(), 'unknown')
99+
})
100+
101+
test('getCallerLocation: no-match', () => {
102+
assert.equal(getCallerLocationFromString('stack\nstring'), 'stack\nstring')
103+
})
104+
105+
test(`getCallerLocationFromString-v8`, () => {
106+
const stack = `
107+
Error
108+
at getCallerLocation (/Users/user/test.js:22:17)
109+
at e (/Users/user/test.js:34:13)
110+
at d (/Users/user/test.js:11:5)
111+
at c (/Users/user/test.js:8:5)
112+
at b (/Users/user/test.js:5:5)
113+
at a (/Users/user/test.js:2:5)
114+
at Object.<anonymous> (/Users/user/test.js:37:1)
115+
at Module._compile (node:internal/modules/cjs/loader:1254:14)
116+
at Module._extensions..js (node:internal/modules/cjs/loader:1308:10)
117+
at Module.load (node:internal/modules/cjs/loader:1117:32)
118+
at Module._load (node:internal/modules/cjs/loader:958:12)
119+
`
120+
assert.match(getCallerLocationFromString(stack), /^.*:11:5.*$/)
121+
})
122+
123+
test(`getCallerLocationFromString-JSC`, () => {
124+
const stack = `
125+
getCallerLocation@/Users/user/test.js:22:17
126+
e@/Users/user/test.js:34:13
127+
d@/Users/user/test.js:11:5
128+
c@/Users/user/test.js:8:5
129+
b@/Users/user/test.js:5:5
130+
a@/Users/user/test.js:2:5
131+
module code@/Users/user/test.js:37:1
132+
evaluate@[native code]
133+
moduleEvaluation@[native code]
134+
moduleEvaluation@[native code]
135+
@[native code]
136+
asyncFunctionResume@[native code]
137+
promiseReactionJobWithoutPromise@[native code]
138+
promiseReactionJob@[native code]
139+
d@/Users/user/test.js:11:5
140+
`
141+
assert.match(getCallerLocationFromString(stack), /^.*:11:5.*$/)
142+
})

test/win32.test.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
// limitations under the License.
1414

1515
import assert from 'node:assert'
16-
import { test, describe, beforeEach } from 'node:test'
16+
import { test, describe } from 'node:test'
1717
import '../build/globals.js'
1818

1919
const _describe = process.platform === 'win32' ? describe : describe.skip

0 commit comments

Comments
 (0)