[Exploratory] Capture good assertion messages.

[jamestalmage]

This just is a quick spike exploring the best way to capture assertion messages.

The only place this does not work great is for wrapOnlyPatterns, since you can end up with a situation where powerAssertContext does not exist even though the args has elements that do not stringify well.

@twada - What do you think about moving this logic into empower-core? Auto populating event.computedMessage or similar?

Maybe we can offer a standard way to map wrapOnlyPatterns to messages?

{
  wrapOnlyPatterns: {
    't.throws(fn, [message])': {
       defaultMessage: 'should throw'
    },
    't.doesNotThrow(fn, [message]': {
        defaultMessage: 'should not throw'
    }
  }
}

// ...

function onAssert(event) {
  var message;
  if (event.enhanced === false) {
     message = event.defaultMessage
  }
}

[sindresorhus]

What do you think about moving this logic into empower-core? Auto populating event.computedMessage or similar?

👍 The less custom code in AVA dealing with this the better :)

[sindresorhus]

What do you think about moving this logic into empower-core? Auto populating event.computedMessage or similar?
Maybe we can offer a standard way to map wrapOnlyPatterns to messages?

@twada ⬆️ Any thoughts?

[twada]

@sindresorhus @jamestalmage Sorry for my late response. I'm a little busy these days.

What do you think about moving this logic into empower-core? Auto populating event.computedMessage or similar?
Maybe we can offer a standard way to map wrapOnlyPatterns to messages?

Sure! Please send a pull-req to empower-core > @jamestalmage

For wrapOnlyPatterns, I do not want to change container type from Array to Object.
So I prefer

  wrapOnlyPatterns: [
    {
      pattern: 't.throws(fn, [message])',
      defaultMessage: 'should throw'
    },
    {
      pattern: 't.doesNotThrow(fn, [message]',
      defaultMessage: 'should not throw'
    }
  ]

to

  wrapOnlyPatterns: {
    't.throws(fn, [message])': {
       defaultMessage: 'should throw'
    },
    't.doesNotThrow(fn, [message]': {
        defaultMessage: 'should not throw'
    }
  }

[sindresorhus]

@jamestalmage Can this be resolved now that empower-core is updated?

[jamestalmage]

Yep. Relevant changes to empower-core are here

[sindresorhus]

Sorry about pinging you all over the place @jamestalmage, but trying to clean up some long-running PR/issues. What's the next step with this PR? Not saying it should be fixed right now. Just would be useful to know the path forward.

[jamestalmage]

No worries!

The issue here would be building up TAP output that would actually show these assertion messages. I think I would like to get a couple other things done first:

1. Bring our tap output more inline with node-tap. Specifically, the yaml output for our errors. This should make the output from all the tap-mocha-reporters much nicer. For example, our errors currently have expected and actual properties, but the tap reporters prefer found and wanted (A bit odd in the JS world, but TAP has it's roots in other programming languages - Perl mostly I think).
2. Integrate clean-yaml-object over serialize-error. I think it will help with 1. Advantages over serialize-error are pretty printing of Buffers, removes large objects attached to some Errors by Domains and EventEmitters, and a few others. I extracted it from node-tap, but haven't gotten around to integrating it.
3. Use tap-emitter. That's really early stages right now. The next step is creating a good way to print subtests. I would love for that to also end up being code shared with node-tap, but that won't be as straightforward as clean-yaml-object or stack-utils were (since it's not directly extracted from node-tap in the first place).

[sindresorhus]

Thanks for the update. Solid plan.

For example, our errors currently have expected and actual properties, but the tap reporters prefer found and wanted

Tape uses actual/expect, so I always assumed that was correct. Doesn't really matter either way. Better to be as compatible with node-tap output as possible.

I would love for that to also end up being code shared with node-tap, but that won't be as straightforward as clean-yaml-object or stack-utils were (since it's not directly extracted from node-tap in the first place).

Awesome that we can share some code with node-tap. Improving those libs will improve both test runners. Hopefully we can share even more code in the future.

Let us know if there's anything @vdemedes and I can help with. Even if it's just feedback on any of those modules.

[jamestalmage]

Yeah sure - I'll file a couple issues on tap-emitter for where I think it should go and we can bikshed there.

[jamestalmage]

RE: actual/expected

Neither is technically "right". The contents of TAPs yaml output is completely unspecified as far as I know. As I recall the fancy mocha style diffs only print when using found / wanted. Maybe it would be better to hunt down where that happens and file a PR so it could use either (i.e. prefer the current properties, but fallback to actual/expected).

[jamestalmage]

Relevant:
https://github.com/isaacs/testanything.github.io/blob/tap14/tap-version-14-specification.md

[sindresorhus]

It's confusing. The v14 draft has examples with both wanted/found and expected/got and expect/got.

// @isaacs

[novemberborn]

Should we track the remaining work in some issues rather than this PR?

[novemberborn]

@jamestalmage perhaps you could open issues to track remaining work, and then close this PR?

[jamestalmage]

Closing as this will never be merged and remaining todo's are tracked in #723