tape-testing / tape

tap-producing test harness for node and browsers
MIT License
5.77k stars 307 forks source link

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

Closed DavidAnson closed 4 years ago

DavidAnson commented 4 years ago
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 https://github.com/substack/tape/commit/11b7d850ffaa8cb679419ff4a0e314b06a3225b9 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 https://github.com/substack/tape/commit/7948e2ee439135efee86ea0b7ce24c9deea4b018.

ljharb commented 4 years ago

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?