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

Default CLI experience should have diff for expected/actual #1412

Open
Krinkle opened this issue Oct 8, 2019 · 2 comments
Open

Default CLI experience should have diff for expected/actual #1412

Krinkle opened this issue Oct 8, 2019 · 2 comments
Labels
Component: CLI Type: Enhancement New idea or feature request.
Milestone

Comments

@Krinkle
Copy link
Member

Krinkle commented Oct 8, 2019

Tell us about your runtime:

  • QUnit version: 2.9.1
  • What environment are you running QUnit in?: Node

What did you expect to happen?

When comparing two objects and a difference is found, for it to at least narrow down where the difference is.

What actually happened?

The default experience uses the TAP reporter which just prints both objects in their entirety.

@Krinkle Krinkle added Type: Enhancement New idea or feature request. Component: CLI labels Oct 8, 2019
@Krinkle
Copy link
Member Author

Krinkle commented Oct 8, 2019

Idea 1 - Standardised diff format provided by test framework

There has been a proposal in the past to standardise the diff as part of the API (#1005, and upstream qunitjs/js-reporters#106).

That would mean QUnit controls how and what generates the diff and feeds it to the reporter. I can imagine some resistance to this as it makes for a rather inflexible interface, e.g. what format would that diff be standardised as, and can we reasonably expect that to be convenient for all kinds of reporting styles and clients? (e.g. plain text, coloured CLI, console, HTML/CSS, possibly something more interactive, etc.)

// QUnit sends to reporter:
{ actual: , expected: , diff: *new* }

Idea 2A - Reporter makes and renders diff

The alternative would be to make the reporter responsible for this. That would mean there is more competition in the space and means there is less duplication between test frameworks. E.g. if you have a good reporter / differ you can connect it to any test framework, that seems useful.

Individual reporters might among them share a common library to provide that functionality, that could still happen.

// Reporter#logError
const diff = makeDiff(actual, expected);
print(diff);

Idea 2B - Reporter handles it - optionally plugged into by test framework / test runner.

Perhaps the JS reporter could provide a hook for providing a diff function. QUnit could feature detect that, and call it with its own comparison/diff logic.

This is not mutually exclusive with idea 2A. If we do both, it means a reporter can choose how it wants to do it (e.g. it could have no own diffing and only implement one of the below formats opted-into by the test runner; or it could have its own diffing and not expose one of the below formats). That might be the most backwards-compatible and yet flexible approaches.

// QUnit optionally provides reporter with a differ
// text/x-diff would e.g. have lines starting with "-", "+" or " ",
// with the reporter colourising these if desired. 
if (reporter.setDiffCallback)
  reporter.setDiffCallback(reporter.DIFF_TEXT, function (actual, expected) {
    return 
  })
// Or, more abstract, and supports intraline diff; based on Google's DiffMatchPatch format.
// A CLI-based reporter would e.g. format these as textual lines,
// using red/green text color per line, and red/green background for inline
// changes. 
reporter.setDiffCallback(reporter.DIFF_OPS, function (actual, expected) {
    return [ [ 0, 'h' ], [ -1, 'a' ], [ 1, 'e' ], [ 0, 'llo'] ];
})

Thoughts?

@Krinkle Krinkle self-assigned this Dec 17, 2019
@trentmwillis
Copy link
Member

I lean towards 2A. Reason being that one of the goals for js-reporters is to provide consistency of output for test frameworks, so it seems reasonable that the reporter should handle generating a diff which is purely an output of the reporter to help developers. Additionally, some reporter types don't show diffs and thus providing it as part of the common interface seems unnecessary.

There's been talk in the past of trying to extract QUnit's diff logic and/or make it pluggable. Seems like that work would fall into part of this ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: CLI Type: Enhancement New idea or feature request.
Development

No branches or pull requests

2 participants