tape-testing / faucet

human-readable TAP summarizer
MIT License
551 stars 25 forks source link

fix(assert): Add default name for no test name. #27

Open alvaropinot opened 6 years ago

alvaropinot commented 6 years ago

First of all I'm a big fan of tape! Thanks for that 😄

I just discovered playing around with a custom TAP reporter for COBOL, (yep COBOL 😅) that whenever a TAP report has no name after the test or the name has no space faucet fails to parse the line.

For example given the following TAP as input:

TAP version 13
1..1
ok 1

or (without the space)

TAP version 13
1..1
ok 1foo
/Users/alvaropinot/work/alvaropinot/faucet/index.js:38
            var s = trim(res.name.trim());
                                 ^

TypeError: Cannot read property 'trim' of undefined
    at Parser.<anonymous> (/Users/alvaropinot/work/alvaropinot/faucet/index.js:38:34)
    at emitOne (events.js:101:20)
    at Parser.emit (events.js:191:7)
    at Parser._online (/Users/alvaropinot/work/alvaropinot/faucet/node_modules/tap-parser/index.js:131:14)
    at Parser._write (/Users/alvaropinot/work/alvaropinot/faucet/node_modules/tap-parser/index.js:53:14)
    at doWrite (/Users/alvaropinot/work/alvaropinot/faucet/node_modules/readable-stream/lib/_stream_writable.js:279:12)
    at writeOrBuffer (/Users/alvaropinot/work/alvaropinot/faucet/node_modules/readable-stream/lib/_stream_writable.js:266:5)
    at Parser.Writable.write (/Users/alvaropinot/work/alvaropinot/faucet/node_modules/readable-stream/lib/_stream_writable.js:211:11)
    at Stream.method [as write] (/Users/alvaropinot/work/alvaropinot/faucet/node_modules/duplexer/index.js:47:39)
    at Socket.ondata (_stream_readable.js:557:20)

This is a fix that prevents the problem but maybe the tap-parser should be fixed instead. I tried to follow the ES5 syntax and already existing style but please feel free to let me know any code standard/style that you want me to follow, if you want to merge this PR.

I was willing to add both scenarios as tests but looks like there is none 😅, quite ironic being a test output lib 😜

Can you confirm if there is any other implications about changing https://github.com/substack/faucet/blob/0fabf821eb142dc75a318c5cd408d7653279bb00/index.js#L38 looks like name is used in some other lines 😄


This are the proposed changes 👇

fix(assert): Add default name for no test name.

ljharb commented 2 years ago

@alvaropinot can you elaborate on what test code produces this output? in latest tape, i get # (anonymous) when i pass null or the empty string as the test name.