Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tape@5 breaks test.comment() in createStream/objectMode context #519

Closed
DavidAnson opened this issue May 21, 2020 · 1 comment
Closed

tape@5 breaks test.comment() in createStream/objectMode context #519

DavidAnson opened this issue May 21, 2020 · 1 comment
Assignees
Labels

Comments

@DavidAnson
Copy link
Contributor

pi@chat:~/tape-bug $ node --version
v14.2.0
pi@chat:~/tape-bug $ ls
repro.js
pi@chat:~/tape-bug $ cat repro.js 
var tape = require("tape");

tape.createStream({ objectMode: true }).on("data", (row) => {
  console.log(JSON.stringify(row))
});

tape("test.comment", (test) => {
  test.comment("message");
  test.end();
});
pi@chat:~/tape-bug $ npm i tape@4
+ tape@4.13.2
pi@chat:~/tape-bug $ node repro.js 
{"type":"test","name":"test.comment","id":0,"skip":false,"todo":false}
"message"
{"type":"end","test":0}
pi@chat:~/tape-bug $ npm i tape@5
+ tape@5.0.0
pi@chat:~/tape-bug $ node repro.js 
{"type":"test","name":"test.comment","id":0,"skip":false,"todo":false}
/home/pi/tape-bug/node_modules/tape/lib/results.js:63
                res.test = id;
                         ^

TypeError: Cannot create property 'test' on string 'message'
    at Test.<anonymous> (/home/pi/tape-bug/node_modules/tape/lib/results.js:63:26)
    at Test.emit (events.js:327:22)
    at Test.bound [as emit] (/home/pi/tape-bug/node_modules/tape/lib/test.js:84:32)
    at /home/pi/tape-bug/node_modules/tape/lib/test.js:149:14
    at forEachArray (/home/pi/tape-bug/node_modules/for-each/index.js:12:17)
    at forEach (/home/pi/tape-bug/node_modules/for-each/index.js:54:9)
    at Test.comment (/home/pi/tape-bug/node_modules/tape/lib/test.js:148:5)
    at Test.bound [as comment] (/home/pi/tape-bug/node_modules/tape/lib/test.js:84:32)
    at Test.<anonymous> (/home/pi/tape-bug/repro.js:8:8)
    at Test.bound [as _cb] (/home/pi/tape-bug/node_modules/tape/lib/test.js:84:32)
pi@chat:~/tape-bug $ 

This functionality is broken by 11b7d85 which adds 'use strict'; to the top of /lib/results.js.

It looks like a longer-standing issue, though. It seems to me that emit('result', res) is meant to pass a result-like object, whereas Test.prototype.comment has treated the comment string as the result object ever since being added in 7948e2e.

@ljharb
Copy link
Collaborator

ljharb commented May 21, 2020

Yes, I agree that strict mode is exposing the underlying, long-standing issue here.

Would you be able/willing to make a PR with this failing test case?

@ljharb ljharb self-assigned this May 24, 2020
@ljharb ljharb closed this as completed in fb94836 May 24, 2020
ljharb added a commit that referenced this issue May 25, 2020
 - [Fix] `createStream`: `result` payload is not always an object (#519)
 - [Fix] Update RegExp for matching stack frames to handle Promise/then scenario (#516)
 - [readme] add `tape-repeater` (#511)
 - [readme] Add link to tape-player (in-process reporter) (#496)
 - [examples] add `ecstatic`
 - [Docs] add an optional emoji version for tap-spec consumer (#501)
 - [Deps] update `minimist`, `resolve`
 - [Tests] Fix simple typo, placehodler -> placeholder (#500)
 - [Dev Deps] update `eslint`, `falafel`, `js-yaml`
ljharb added a commit that referenced this issue May 25, 2020
v4.13.3

 - [Fix] `createStream`: `result` payload is not always an object (#519)
 - [Fix] Update RegExp for matching stack frames to handle Promise/then scenario (#516)
 - [readme] add `tape-repeater` (#511)
 - [readme] Add link to tape-player (in-process reporter) (#496)
 - [examples] add `ecstatic`
 - [Docs] add an optional emoji version for tap-spec consumer (#501)
 - [Deps] update `minimist`, `resolve`
 - [Tests] Fix simple typo, placehodler -> placeholder (#500)
 - [Dev Deps] update `eslint`, `falafel`, `js-yaml`
ljharb added a commit that referenced this issue May 25, 2020
 - [Fix] `createStream`: `result` payload is not always an object (#519)
 - [Fix] Update RegExp for matching stack frames to handle Promise/then scenario (#515)
 - [readme] add `tape-repeater` (#511)
 - [Dev Deps] update `eslint`
ljharb added a commit that referenced this issue Dec 29, 2020
 - [New] Include name of test in log when test times out (#524)
 - [readme] document Promise support; remove Promise-related alternatives
 - [readme] add `tape-describe` to 'other' section (#523)
 - [Deps] update `deep-equal`, `is-regex`, `object-inspect`, `object-is`, `object.assign`, `resolve`, `string.prototype.trim`
 - [Dev Deps] update `eslint`, `js-yaml`
 - [eslint] remove useless regex escapes
 - [Tests] handle stack differences in node 15
 - [Tests] add test case for #519 for test.comment() in createStream/objectMode context (#520)
ljharb added a commit that referenced this issue Jul 28, 2021
 - [New] add `.teardown()` on `t` instances (#546)
 - [New] Include name of test in log when test times out (#524)
 - [Refactor] avoid reassigning arguments
 - [Refactor] remove unused line, unneeded var initialization; add missing `new`
 - [Refactor] remove use of legacy `exports`
 - [Refactor] Avoid setting message property on primitives; use strict mode to catch this
 - [Refactor] generalize error message from calling `.end` more than once
 - [Refactor] use `call-bind/callBound` instead of `function-bind` directly
 - [readme] improve `t.throws` documentation (#541)
 - [readme] Another way to create custom reportersA (#556)
 - [readme] remove long-dead testling-ci badge
 - [readme] add `tape-describe` to 'other' section (#523)
 - [readme] remove travis badge; add actions and codecov badges
 - [eslint] remove useless regex escapes
 - [eslint] fully enable `@ljharb` eslint config
 - [meta] do not publish github action workflow files
 - [meta] add `safe-publish-latest`; use `prepublishOnly` script for npm 7+
 - [meta] run `aud` in `posttest`
 - [Deps] update `glob`, `is-regex`, `object-inspect`, `resolve`, `string.prototype.trim`
 - [Dev Deps] update `eslint`
 - [actions] use `node/install` instead of `node/run`; use `codecov` action
 - [Tests] exclude examples from coverage
 - [Tests] ensure bin/tape is linted
 - [Tests] make `stripFullStack` output an array of lines, for better failure messages
 - [Tests] handle stack differences in node 15
 - [Tests] add test case for #519 for test.comment() in createStream/objectMode context
ljharb added a commit that referenced this issue Jul 28, 2021
v4.14.0

 - [New] add `.teardown()` on `t` instances (#546)
 - [New] Include name of test in log when test times out (#524)
 - [Refactor] avoid reassigning arguments
 - [Refactor] remove unused line, unneeded var initialization; add missing `new`
 - [Refactor] remove use of legacy `exports`
 - [Refactor] Avoid setting message property on primitives; use strict mode to catch this
 - [Refactor] generalize error message from calling `.end` more than once
 - [Refactor] use `call-bind/callBound` instead of `function-bind` directly
 - [readme] improve `t.throws` documentation (#541)
 - [readme] Another way to create custom reportersA (#556)
 - [readme] remove long-dead testling-ci badge
 - [readme] add `tape-describe` to 'other' section (#523)
 - [readme] remove travis badge; add actions and codecov badges
 - [eslint] remove useless regex escapes
 - [eslint] fully enable `@ljharb` eslint config
 - [meta] do not publish github action workflow files
 - [meta] add `safe-publish-latest`; use `prepublishOnly` script for npm 7+
 - [meta] run `aud` in `posttest`
 - [Deps] update `glob`, `is-regex`, `object-inspect`, `resolve`, `string.prototype.trim`
 - [Dev Deps] update `eslint`
 - [actions] use `node/install` instead of `node/run`; use `codecov` action
 - [Tests] exclude examples from coverage
 - [Tests] ensure bin/tape is linted
 - [Tests] make `stripFullStack` output an array of lines, for better failure messages
 - [Tests] handle stack differences in node 15
 - [Tests] add test case for #519 for test.comment() in createStream/objectMode context
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants