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

rename or alias td.explain().description to .message #314

Closed
jasonkarns opened this issue Dec 23, 2017 · 6 comments
Closed

rename or alias td.explain().description to .message #314

jasonkarns opened this issue Dec 23, 2017 · 6 comments

Comments

@jasonkarns
Copy link
Member

This would allow throw td.explain(foo) to print in a naturally helpful way.

Also thinking the explain return value should have description == .toString()

Thoughts?

@searls
Copy link
Member

searls commented Dec 24, 2017

Is throwing any ol' not-error-but-object-that-has-a-message-property some legit pattern that I hadn't heard about?

@jasonkarns
Copy link
Member Author

jasonkarns commented Dec 27, 2017

Depends on your definition of "legit". Have seen a few error modules that define custom Error classes. I think most test utility AssertionError type objects are in the same boat. (only the "good" ones actually set their prototype to Error)

But beyond that, just seems like legit duck typing to me, since most test frameworks (teenytest included) expect to receive some form of assertion error or Error-like object. But they don't type-check against Error; instead just printing .message in a formatted way.

Anyway, my use-case:

Whenever needing td.explain, I'm generally console-logging the .description property to the screen. Which in most test frameworks causes the explain output to hit the console before the test description which generated the log. Or, if the explain needs logged from within the SUT, then it is getting logged by numerous tests, and it's mildly confusing which test the output corresponds to. (since they're still printed ahead of the test name, and interleaved)

I generally find it easier to just throw td.explain(x) instead of console.log(td.explain(x).description). The throw variant is shorter; the throw keyword sticks out and is guaranteed to be removed and not get left in the test accidentally, unlike console-logs; and most importantly, the description gets logged after the test title so it's a bit more clear.

Seems like a nice extra affordance with not too much downside. (Well, other than the eventual migration from description to message, since aliases kinda suck.)

@searls
Copy link
Member

searls commented Jan 5, 2018

ah, clever! I think that makes sense. Have you spiked to confirm that this would work?

@jasonkarns
Copy link
Member Author

jasonkarns commented Jan 6, 2018

Sample test output when console-logging the td.explain output:

ok 9 - "Some test" - test #7 in `test/lib/index.js`
This test double `../../src/data: .default` has 0 stubbings and 2 invocations.

Invocations:
  - called with `({})`.
  - called with `({foo: "bar"})`.
ok 10 - "A test that throws td.explain" - test #1 in `test/lib/index.js`
ok 11 - "Another test" - test #2 in `test/lib/index.js`

Same test but throwing the td.explain object (note where the output goes in relation to test num 10:

ok 9 - "Some test" - test #7 in `test/lib/index.js`
not ok 10 - "A test that throws td.explain" - test #1 in `test/lib/index.js`
  ---
  message: undefined
  stacktrace: undefined
  ...
ok 11 - "Another test" - test #2 in `test/lib/index.js`

After simply changing description to message on the td.explain return object:

ok 9 - "Some test" - test #7 in `test/lib/index.js`
not ok 10 - "A test that throws td.explain" - test #1 in `test/lib/index.js`
  ---
  message: This test double `../../src/data: .default` has 0 stubbings and 2 invocations.

Invocations:
  - called with `({})`.
  - called with `({foo: "bar"})`.
  stacktrace: undefined
  ...
ok 11 - "Another test" - test #2 in `test/lib/index.js`

It definitely works as desired. The console.log(td.explain(double).description) results in the nicest output, but requires the most boilerplate, compared to throw td.explain(double). At the cost of having a bit of yaml-ish formatting (ie the --- preamble, and the message:/stacktrace: prefixes.

Evidently, chrome/node's console.log method does not invoke toString, nor valueOf, nor toJSON on any of its arguments, so at present there doesn't seem to be much value in adding any of those methods. However, node does invoke .inspect for any console.log argument that has a .inspect method... So, adding: inspect() { return this.description } to the explain object, and then console logging as: console.log(td.explain(someDouble)) results in the same thing as: console.log(td.explain(someDouble).description) (Additionally, the inspect method is given the current indentation depth and some options like showHidden and colors (to colorize the output with ansi codes. This could be a really cool feature for custom test reporters...)

@searls
Copy link
Member

searls commented Jan 6, 2018

Dayum, that is some interesting detail on Node's console's inspect contract. We should totally take advantage of this for td.explain

@jasonkarns jasonkarns added this to Backlog in Test Double Trouble Oct 1, 2021
@giltayar
Copy link
Collaborator

Stale. Closing. Please reopen if still relevant and I will look into it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

3 participants