[Fix] emit skipped tests as objects

[r0mflip]

When using objectMode skipped tests are emitted as comments, they are actual tests that need to be emitted as objects.

Test code:

const test = require('tape');
const printer = new require('stream').Writable({objectMode: true});
printer.write = printer.end = obj => {
  console.log(obj);
};

test.createStream({objectMode: true})
.pipe(printer);

test('parent', t => {
  t.equal(true, true);
  t.test('child1', {skip: 'not installed'}, t2 => {
    t2.equal(true, true);
    t2.equal(true, false);
    t2.end();
  });
  t.test('child2', {todo: 'will get done soon'}, t3 => {
    t3.equal(true, false);
    t3.equal(true, true);
    t3.end();
  });
  t.test('child3', {todo: 'will do'});
  t.equal(true, true);
  t.equal(true, true);
  t.end();
});

When used normally:

{ type: 'test', name: 'parent', id: 0 }
{
  id: 0,
  ok: true,
  skip: undefined,
  todo: false,
  name: 'should be equal',
  operator: 'equal',
  objectPrintDepth: 5,
  actual: true,
  expected: true,
  test: 0,
  type: 'assert'
}
{
  id: 1,
  ok: true,
  skip: undefined,
  todo: false,
  name: 'should be equal',
  operator: 'equal',
  objectPrintDepth: 5,
  actual: true,
  expected: true,
  test: 0,
  type: 'assert'
}
{
  id: 2,
  ok: true,
  skip: undefined,
  todo: false,
  name: 'should be equal',
  operator: 'equal',
  objectPrintDepth: 5,
  actual: true,
  expected: true,
  test: 0,
  type: 'assert'
}
SKIP child1                                                 <- plain text
{ type: 'end', test: 1 }                                    <- no object with testId 1
{ type: 'test', name: 'child2', id: 2, parent: 0 }
{
  id: 0,
  ok: false,
  skip: undefined,
  todo: 'will get done soon',
  name: 'should be equal',
  operator: 'equal',
  objectPrintDepth: 5,
  actual: true,
  expected: false,
  functionName: 'Test.<anonymous>',
  file: 'D:\\dev\\testing\\tape-test\\test.js:19:8',
  line: 19,
  column: 8,
  at: 'Test.<anonymous> (D:\\dev\\testing\\tape-test\\test.js:19:8)',
  test: 2,
  type: 'assert'
}
{
  id: 1,
  ok: true,
  skip: undefined,
  todo: 'will get done soon',
  name: 'should be equal',
  operator: 'equal',
  objectPrintDepth: 5,
  actual: true,
  expected: true,
  test: 2,
  type: 'assert'
}
{ type: 'end', test: 2 }
{ type: 'end', test: 3 }                                       <- no object with testId 3
{ type: 'end', test: 0 }

After patch:

{ type: 'test', name: 'parent', id: 0, skip: false, todo: false }
{
  id: 0,
  ok: true,
  skip: undefined,
  todo: false,
  name: 'should be equal',
  operator: 'equal',
  objectPrintDepth: 5,
  actual: true,
  expected: true,
  test: 0,
  type: 'assert'
}
{
  id: 1,
  ok: true,
  skip: undefined,
  todo: false,
  name: 'should be equal',
  operator: 'equal',
  objectPrintDepth: 5,
  actual: true,
  expected: true,
  test: 0,
  type: 'assert'
}
{
  id: 2,
  ok: true,
  skip: undefined,
  todo: false,
  name: 'should be equal',
  operator: 'equal',
  objectPrintDepth: 5,
  actual: true,
  expected: true,
  test: 0,
  type: 'assert'
}
{
  type: 'test',
  name: 'child1',
  id: 1,                                                         <- object with testId 1
  skip: 'not installed',
  todo: false,
  parent: 0
}
{ type: 'end', test: 1 }                                         <- reference exists
{
  type: 'test',
  name: 'child2',
  id: 2,
  skip: false,
  todo: 'will get done soon',
  parent: 0
}
{
  id: 0,
  ok: false,
  skip: undefined,
  todo: 'will get done soon',
  name: 'should be equal',
  operator: 'equal',
  objectPrintDepth: 5,
  actual: true,
  expected: false,
  functionName: 'Test.<anonymous>',
  file: 'D:\\dev\\testing\\tape-test\\test.js:19:8',
  line: 19,
  column: 8,
  at: 'Test.<anonymous> (D:\\dev\\testing\\tape-test\\test.js:19:8)',
  test: 2,
  type: 'assert'
}
{
  id: 1,
  ok: true,
  skip: undefined,
  todo: 'will get done soon',
  name: 'should be equal',
  operator: 'equal',
  objectPrintDepth: 5,
  actual: true,
  expected: true,
  test: 2,
  type: 'assert'
}
{ type: 'end', test: 2 }
{
  type: 'test',
  name: 'child3',
  id: 3,                                                         <- object with testId 3
  skip: false,
  todo: 'will do',
  parent: 0
}
{ type: 'end', test: 3 }                                         <- reference exists
{ type: 'end', test: 0 }

[ljharb]

Please add tests covering your new changes.

[r0mflip]

Please add tests covering your new changes.

Will add tests for objectMode stream and stuff, if all regarding changes in this PR are ok

[ljharb]

Please do; tests help review of code.

[r0mflip]

Node 0.8 has a weird issue that I don't understand why only it fails.

[ljharb]

the test on 0.8 seems to want Test.tap.test.test.todo here instead of Test.<anonymous> :-/

I'm confused because we have many other tests using Test.<anonymous> that don't have special handling for node 0.8.

[r0mflip]

I get that, but only why node 0.8 😭 I've gone through the source and debugged it a thousand times but only node 0.8 failes for only todo_explanation.js. It's just a copy of todo.js

[ljharb]

I'm wondering if the change to `emit('prerun') somehow changed things for the v8 version 0.8 is using.

We could update test/common, perhaps, for node 0.8 only, to swap out Test.tap.test.test.todo for Test.<anonymous>?

[r0mflip]

I'm wondering if the change to `emit('prerun') somehow changed things for the v8 version 0.8 is using.

I tried by using the latest commit from master, It still has the same problem. It works normally, this PR effects it somehow.

My [naive] understanding is that in V8 version of 0.8, the stack part assumes the {todo: 'something'} object as one of the stack layer. The same happens for my local skip_explanation.js test. The test in #473 (comment) comment has the function in stack pointing to Test.tap.test.platform.

Update: IDK but when I pass strings as arguments to todo or skip this happens, when they are booleans it totally works.

[r0mflip]

Removed loose patches. Current changes include:

1. Emit all objects (tests added)
2. Add prefix (# TODO ..., # SKIP ...) to comments (tests altered accordingly)

Will try to add descriptive messages in another PR

[ljharb]

Looks great as a bugfix.

[ljharb]

apparently, the through package has never supported an objectMode option, so i'm not sure what this is testing.

[Raynos]

objectMode does not exist as an option here, but the implementation is so trivial that it works with strings, objects & buffers by default. Where as node streams require opting in to allow objects with an objectMode flag.

[ljharb]

makes sense, the presence of the unused option here confused me, is all :-)