Exposes generation of diffs from base reporter #3213
Conversation
|
Looks good to me. @harrysarson thanks! |
|
I'll update IntelliJ reporter once the PR is merged. |
There was a problem hiding this comment.
This looks good, thanks @harrysarson. I just have a couple nitpicks.
@segrey To make sure I'm not missing something, can you clarify what you meant about err.details back in #3201?
lib/reporters/base.js
Outdated
| var generateDiff = exports.generateDiff = function (actual, expected) { | ||
| if (exports.inlineDiffs) { | ||
| return inlineDiff(actual, expected); | ||
| } else { |
There was a problem hiding this comment.
this else is not necessary
lib/reporters/base.js
Outdated
| * The diff will be either inline or unified dependant on the value | ||
| * of `Base.inlineDiff`. | ||
| * | ||
| * @api private |
There was a problem hiding this comment.
I think the intent is that this is a public function, so you can remove this, right?
lib/reporters/base.js
Outdated
| * of `Base.inlineDiff`. | ||
| * | ||
| * @api private | ||
| * @param {String} actual |
test/reporters/base.spec.js
Outdated
| var oldInlineDiffs = Base.inlineDiffs; | ||
| Base.inlineDiffs = false; | ||
|
|
||
| var output = Base.generateDiff(actual, expected); |
There was a problem hiding this comment.
please do the patching in a beforeEach of the suite and restore in afterEach
There was a problem hiding this comment.
When doing the patching, should I be setting the value of Base.inlineDiffs in the beforeEach?
boneskull
added
semver-patch
|
@harrysarson Thanks for this!! in the future, please just reuse your PRs and force-push changes--it's kind of difficult to track the conversation otherwise |
|
@boneskull Regarding
As a benefit, mocha will have a mechanism to easily change how these values are calculated, e.g. supply diffs provided by assertion libraries without changes in reporters. |
|
I have hopefully made all the requested changes :)
Ah yeah, I will in future. |
|
Now fixed |
harrysarson
force-pushed
the
seperate-diff2
branch
from
e159bf1 to
f90a3ce
Compare
|
@boneskull can this be merged or is more work needed, I think I have resolved all the required changes :) |
|
@segrey Got it. So it looks like this PR is the first step to get there. |
|
blah, meant to squash. |
Second attempt at solution for #3011, improves on PR #3201.
PR based on suggestion by @segrey in #3201 (comment). Exposed function
generateDiff(expected, actual)as a static member ofBase.The value of
Base.inlineDiffsis used to determine the form of the diff.Alternate Designs
#3201
Why should this be in core?
Allows IDE's to produce nice output when integrated with mocha.
Benefits
See #3011.