Skip to content

Commit

Permalink
refactor: enhance stack from (#752)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>
Co-authored-by: Min Idzelis <[email protected]>
  • Loading branch information
3 people authored Mar 28, 2024
1 parent b02fd52 commit 2026d4a
Show file tree
Hide file tree
Showing 7 changed files with 113 additions and 8 deletions.
19 changes: 19 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,22 @@ jobs:
timeout-minutes: 1
env:
FORCE_COLOR: 3

bun:
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v3
- name: Use Node.js 20.x
uses: actions/setup-node@v3
with:
node-version: 20.x
- name: Setup Bun
uses: antongolub/action-setup-bun@v1

- run: npm ci
- run: npm run build
- run: bun test ./test/bun.test.js
timeout-minutes: 1
env:
FORCE_COLOR: 3
3 changes: 2 additions & 1 deletion src/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
errnoMessage,
exitCodeInfo,
formatCmd,
getCallerLocation,
noop,
parseDuration,
quote,
Expand Down Expand Up @@ -128,7 +129,7 @@ export const $: Shell & Options = new Proxy<Shell & Options>(
})
}
}
const from = new Error().stack!.split(/^\s*at\s/m)[2].trim()
const from = getCallerLocation()
if (pieces.some((p) => p == undefined)) {
throw new Error(`Malformed command at ${from}`)
}
Expand Down
13 changes: 13 additions & 0 deletions src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -355,3 +355,16 @@ const reservedWords = [
'done',
'in',
]

export function getCallerLocation(err = new Error()) {
return getCallerLocationFromString(err.stack)
}

export function getCallerLocationFromString(stackString = 'unknown') {
return (
stackString
.split(/^\s*(at\s)?/m)
.filter((s) => s?.includes(':'))[2]
?.trim() || stackString
)
}
29 changes: 29 additions & 0 deletions test/bun.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright 2024 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// https://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

import assert from 'node:assert'
import { test, describe } from 'bun:test'
import '../build/globals.js'

describe('bun', () => {
test('smoke test', async () => {
const p = await $`echo foo`
assert.match(p.stdout, /foo/)
})

test('captures err stack', async () => {
const p = await $({ nothrow: true })`echo foo; exit 3`
assert.match(p.message, /exit code: 3/)
})
})
7 changes: 1 addition & 6 deletions test/cli.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
// limitations under the License.

import assert from 'node:assert'
import { test, describe, before, beforeEach } from 'node:test'
import { test, describe, beforeEach } from 'node:test'
import '../build/globals.js'

describe('cli', () => {
Expand Down Expand Up @@ -70,11 +70,6 @@ describe('cli', () => {
assert.match(out.stdout, /verbose is false/)
})

test('supports `--experimental` flag', async () => {
let out = await $`echo 'echo("test")' | node build/cli.js --experimental`
assert.match(out.stdout, /test/)
})

test('supports `--quiet` flag', async () => {
let p = await $`node build/cli.js --quiet test/fixtures/markdown.md`
assert.ok(!p.stderr.includes('ignore'), 'ignore was printed')
Expand Down
48 changes: 48 additions & 0 deletions test/util.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
quote,
quotePowerShell,
randomId,
getCallerLocationFromString,
} from '../build/util.js'

describe('util', () => {
Expand Down Expand Up @@ -92,3 +93,50 @@ describe('util', () => {
)
})
})

test('getCallerLocation: empty', () => {
assert.equal(getCallerLocationFromString(), 'unknown')
})

test('getCallerLocation: no-match', () => {
assert.equal(getCallerLocationFromString('stack\nstring'), 'stack\nstring')
})

test(`getCallerLocationFromString-v8`, () => {
const stack = `
Error
at getCallerLocation (/Users/user/test.js:22:17)
at e (/Users/user/test.js:34:13)
at d (/Users/user/test.js:11:5)
at c (/Users/user/test.js:8:5)
at b (/Users/user/test.js:5:5)
at a (/Users/user/test.js:2:5)
at Object.<anonymous> (/Users/user/test.js:37:1)
at Module._compile (node:internal/modules/cjs/loader:1254:14)
at Module._extensions..js (node:internal/modules/cjs/loader:1308:10)
at Module.load (node:internal/modules/cjs/loader:1117:32)
at Module._load (node:internal/modules/cjs/loader:958:12)
`
assert.match(getCallerLocationFromString(stack), /^.*:11:5.*$/)
})

test(`getCallerLocationFromString-JSC`, () => {
const stack = `
getCallerLocation@/Users/user/test.js:22:17
e@/Users/user/test.js:34:13
d@/Users/user/test.js:11:5
c@/Users/user/test.js:8:5
b@/Users/user/test.js:5:5
a@/Users/user/test.js:2:5
module code@/Users/user/test.js:37:1
evaluate@[native code]
moduleEvaluation@[native code]
moduleEvaluation@[native code]
@[native code]
asyncFunctionResume@[native code]
promiseReactionJobWithoutPromise@[native code]
promiseReactionJob@[native code]
d@/Users/user/test.js:11:5
`
assert.match(getCallerLocationFromString(stack), /^.*:11:5.*$/)
})
2 changes: 1 addition & 1 deletion test/win32.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
// limitations under the License.

import assert from 'node:assert'
import { test, describe, beforeEach } from 'node:test'
import { test, describe } from 'node:test'
import '../build/globals.js'

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

0 comments on commit 2026d4a

Please sign in to comment.