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

fix stack where actual==falsy #400

Merged
merged 1 commit into from
Oct 18, 2017
Merged

Conversation

nhamer
Copy link
Contributor

@nhamer nhamer commented Oct 7, 2017

Fixes #399

lib/results.js Outdated
@@ -157,7 +157,7 @@ function encodeResult (res, count) {
output += inner + 'at: ' + res.at + '\n';
}

var actualStack = res.actual && res.actual.stack;
var actualStack = res.actual !== undefined && res.actual.stack;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will cause it to throw if res.actual is null.

If res.actual is a primitive (even a truthy primitive), .stack will never have a meaningful value.

This check should probably really be res.actual && (typeof res.actual === 'object' || typeof res.actual === 'function') ? res.actual.stack : null

Copy link
Contributor Author

@nhamer nhamer Oct 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm working slightly [confused] here, on windows a lot of the unit tests fail. Will update the pr.

Copy link
Contributor Author

@nhamer nhamer Oct 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see. It was working on my machine for the wrong reason. The actual bug is:

when res.actual === false,
(res.actual && res.actual.stack) === false, not undefined

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nhamer fyi, i edited your comment to remove the inappropriate word.

@ljharb ljharb changed the title fix stack where actual==falsey fix stack where actual==falsy Oct 8, 2017
Copy link
Collaborator

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the clarification. Could you provide a test that would fail without this change?

@nhamer
Copy link
Contributor Author

nhamer commented Oct 9, 2017

it was the inverse of the existing test; fixed the existing test's assertion to accurately reflect its arguments

@ljharb ljharb merged commit 10b7dcd into tape-testing:master Oct 18, 2017
@ljharb ljharb added the bug label Oct 18, 2017
@nhamer nhamer deleted the actual_is_falsey branch October 18, 2017 14:01
ljharb added a commit that referenced this pull request Feb 19, 2018
 - [New] use `process.env.NODE_TAPE_OBJECT_PRINT_DEPTH` for the default object print depth (#420)
 - [New] Add "onFailure" listener to test harness (#408)
 - [Fix] fix stack where actual is falsy (#400)
 - [Fix] normalize path separators in stacks (#402)
 - [Fix] fix line numbers in stack traces from inside anon functions (#387)
 - [Fix] Fix dirname in stack traces (#388)
 - [Robustness] Use local reference for clearTimeout global (#385)
 - [Deps] update `function-bind`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants