Skip to content
This repository has been archived by the owner on Nov 8, 2024. It is now read-only.

Redesign CLI reporter #765

Open
honzajavorek opened this issue Apr 7, 2017 · 12 comments
Open

Redesign CLI reporter #765

honzajavorek opened this issue Apr 7, 2017 · 12 comments
Labels
Epic: CLI reporter Dredd's default reporter

Comments

@honzajavorek
Copy link
Contributor

honzajavorek commented Apr 7, 2017

Dredd's CLI reporter, which is what most users see as its default output, has several flaws. You can list some of them using the CLI reporter label. The aim of this meta-issue is to design a new reporter.

Requirements / Internal Redesign

  • Use debug for Dredd application logging. Separate logging from the CLI reporter completely.

CLI Reporter Redesign

Progress

Print short progress information first, use ✔ or number in case of a failure. Make the progress information very brief. Something like:

Machines > Machines collection > Create Machines
  → POST /path/to/something (application/json)
  ← 200 (application/json)
  ✔ PASS ... 234ms

Machines > Machines collection > Get Machines
  → GET /path/to/something?limit=34
  ← 200 (application/json)
  1) FAILURE ... 34ms

If possible (terminal support, --color / --no-color), use colors (green, red) to denote pass or failure. The requested URL displayed in the output is the real one, with templates filled, with all changes applied, not the one from API description (#405). Every test starts with a transaction name (#430).

List of Failures

List failures at the end, reference by failure numbers. Each failure in the list of failures contains:

  • Transaction name
  • Details about the request made
    • Method, headers, body used for the request
    • Filename of the API description document with :line-number at the end (clickable on some terminals, can be opened in an editor)
    • Idea: Information (yes / no) whether the transaction was modified in hooks or whether it was left intact as specified in the API description
    • Idea: Debugging hint which curl command would perform exactly the same HTTP request as Dredd did during testing.
  • Details about the failure and explanation of the error. All common errors (JSON Schema validation errors, unparseable JSON errors, APIB/Swagger parse errors and warnings, ...) should be presented in such way that user of Dredd clearly has an idea on what happened, what is the problem and what to do next. (See examples of unreadable failed tests in body: At '' Invalid type: object (expected array) #745, Improve error messages coming from invalid JSON parsing #665)
    • Clearly distinguish between failures (non-passing tests) and errors (the test errored, there's stack trace or something).
  • Colored diff for the response (currently there is no diff, Dredd displays expected response and the real response and leaves it up to the user to do the diff in their head)
    • Status codes - just two colored numbers next to each other, no extra text
    • Headers - the difference, colored
    • Bodies - the difference, colored
      • If the body is large and very different, possibly with different Content Types (like small JSON vs huge HTML), just cut it, no point in displaying it all (user doesn't need to know what's exactly wrong when it's very clear it's all wrong).
      • If the difference is only in whitespace, detect such case and display the whitespace! (Display whitespace in diff using visible characters #305)
      • If the difference is only in newline at the end and the used format is API Blueprint, inform about trailing newline fails text/plain #67 (it's the least we can do to mitigate the problem).
========================================================================
1)  Machines > Machines collection > Get Machines
    /Users/honza/myapi/apiary.apib:45

-- FAILURES ------------------------------------------------------------

Status code is not '200'

Header 'content-type' has value 'text/html; charset=utf-8' instead
of 'application/json; charset=utf-8'

Can't validate real media type 'text/plain' against expected media
type 'application/json; charset=utf-8'.   

-- REQUEST -------------------------------------------------------------

POST /path/to/something?limit=34

Header: value
X-Header2: value2
Content-Type: application/json

{
   "id": 42,
   "hello": true
}

-- RESPONSE ------------------------------------------------------------

404 ≠ 200

Missing-header: blah blah
X-Header2: value2

{
   "id": 42,
   "diff": "everywhere"
}

-- SERVER OUTPUT -------------------------------------------------------

[Apr 7 11:51:39 2016] 127.0.0.1:44062 [200]: /path/to/something?limit=34
[Apr 7 11:51:39 2016] 127.0.0.1:44062 [200]: /path/to/something?limit=34
[Apr 7 11:51:39 2016] 127.0.0.1:44062 [200]: /path/to/something?limit=34
[Apr 7 11:51:39 2016] 127.0.0.1:44062 [200]: /path/to/something?limit=34

Idea from @kylef: Another section could be a snippet of the source API description with the expectation which failed.

Summary

Lists passed, failed, errorred, skipped tests. If any of these categories is zero, it's not listed.

Tests:       1 passed, 1 failed, 1 total
Time:        0.565s

The # total part is always listed (so if Dredd finds 0 tests, it's going to be:

Tests:       0 total
Time:        0.2s

Fixes

Documentation

  • Special documentation page about reporters and how they work.
  • Document Dredd exit codes

Related New Features / Follow-Ups

  • If --server is used, capture the server output. Silence it by default. If tests fail, print it separately (see pytest). Allow to force certain behavior by CLI options. Resolves Dredd redirects server output #593.
  • If hooks are used, capture the hooks handler output. Silence it by default. If the tests fail and the hooks handler process ends in any other way than with zero exit status, print it separately (see pytest). Allow to force certain behavior by CLI options.
  • Add option to stop after the first failure / error.
  • If Apiary Reporter is used, Dredd could open it right away for users (very simple using the opener package).
  • Idea: Support the TAP protocol? https://testanything.org/, https://github.com/sindresorhus/awesome-tap (this way we could make Dredd easily extensible and we could offload some built-in reporters to existing TAP tooling)
@honzajavorek honzajavorek added Epic: CLI reporter Dredd's default reporter improvement labels Apr 7, 2017
@driesvints
Copy link
Contributor

This looks good. What I'm currently sorely missing is a summarized way to see passed/failed requests.

Something like:

$ dredd spec.apib http://0.0.0.0:3000/api --level short
pass: GET /events
pass: GET /events/1
pass: DELETE /events/1
pass: DELETE /events/1
complete: 4 passing, 0 failing, 0 errors, 0 skipped, 4 total
Done in 10.00s.

Or perhaps this? This is even better because this way you can distinct the two examples for the same METHOD call on an endpoint.

$ dredd spec.apib http://0.0.0.0:3000/api --level short
pass: Events > Events collection > Get Events
pass: Events > Events collection > Get Event
pass: Events > Events collection > Delete Event > Example 1
pass: Events > Events collection > Delete Event > Example 2
complete: 4 passing, 0 failing, 0 errors, 0 skipped, 4 total
Done in 10.00s.

With failures showing at the end as you proposed above?

@honzajavorek
Copy link
Contributor Author

@driesvints So the

Machines > Machines collection > Create Machines
  → POST /path/to/something (application/json)
  ← 200 (application/json)
  ✔ PASS ... 234ms

output would still be way too verbose for you?

@driesvints
Copy link
Contributor

driesvints commented Apr 12, 2017

That actually looks a lot better than the current implementation. But I think an extra level which reduces it to a single line could also have its purpose (in CI outputs etc).

It's definitely not something which needs to happen for the first release, perhaps we can work on this later and continue with the above proposal now?

@driesvints
Copy link
Contributor

driesvints commented Apr 12, 2017

Or even a dot level so you get something like this:

..... 5/5

complete: 4 passing, 0 failing, 0 errors, 0 skipped, 4 total
Done in 10.00s.

This is especially handy in CLI reports.

@honzajavorek
Copy link
Contributor Author

@driesvints There already is a dot reporter: dredd --reporter=dot However, I can imagine dredd --reporter=line as well 😄

@driesvints
Copy link
Contributor

ah yeah but you can't use that in combination with sending reports to apiary right?

@honzajavorek
Copy link
Contributor Author

You should be, Dredd supports multiple reporters.

@driesvints
Copy link
Contributor

driesvints commented Apr 12, 2017

You should be, Dredd supports multiple reporters.

TIL

Bert

@clementgarnier
Copy link

Hi there!

Regarding exit codes, I've noticed that dredd returns 0 even when tests are errored (e.g. when the server didn't start). Is that intentional?

Thank you!

@honzajavorek
Copy link
Contributor Author

@clementgarnier That is definitely not intentional. Could you please file a separate issue for that? This would be a serious error and should be fixed right away.

@honzajavorek
Copy link
Contributor Author

I found some old notes we should eventually take into account:

Show transaction names and blueprint source file in Dredd CLI reporter, show summary of errors at the end of report without payloads.

@honzajavorek
Copy link
Contributor Author

Note: We should not forget about separating (or not) hook handlers output as well - #910 (comment)

@honzajavorek honzajavorek added this to the Improve CLI output milestone Jan 12, 2019
honzajavorek added a commit that referenced this issue Jan 17, 2019
A part of the effort to separate application logging from the reporters output.

Addresses #1089, supersedes #1099, enables #765

BREAKING CHANGE: Instead of --level use --loglevel. The option is no longer able to affect reporter output, it only affects application logging output. Use --loglevel=silent instead of --silent, which is now removed. The --loglevel option now only accepts 'silent', 'error', 'warning', 'debug' as values.
honzajavorek added a commit that referenced this issue Jan 17, 2019
A part of the effort to separate application logging from the reporters output.

Enables #1089 and subsequently #765, supersedes #1099

BREAKING CHANGE: The --level option is no longer able to affect reporter output, it only affects application logging output.
honzajavorek added a commit that referenced this issue Jan 17, 2019
A part of the effort to separate application logging from the reporters output.

Addresses #1089, supersedes #1099, enables #765

BREAKING CHANGE: Instead of --level use --loglevel. The option is no longer able to affect reporter output, it only affects application logging output. Use --loglevel=silent instead of --silent, which is now removed. The --loglevel option now only accepts 'silent', 'error', 'warning', 'debug' as values.
honzajavorek added a commit that referenced this issue Jan 17, 2019
A part of the effort to separate application logging from the reporters output.

Enables #1089 and subsequently #765, supersedes #1099

BREAKING CHANGE: The --level option is no longer able to affect reporter output, it only affects application logging output.
honzajavorek added a commit that referenced this issue Jan 17, 2019
A part of the effort to separate application logging from the reporters output.

Addresses #1089, supersedes #1099, enables #765

BREAKING CHANGE: Instead of --level use --loglevel. The option is no longer able to affect reporter output, it only affects application logging output. Use --loglevel=silent instead of --silent, which is now removed. The --loglevel option now only accepts 'silent', 'error', 'warning', 'debug' as values.
honzajavorek added a commit that referenced this issue Jan 23, 2019
A part of the effort to separate application logging from the reporters output.

Enables #1089 and subsequently #765, supersedes #1099

BREAKING CHANGE: The --level option is no longer able to affect reporter output, it only affects application logging output.
honzajavorek added a commit that referenced this issue Jan 23, 2019
A part of the effort to separate application logging from the reporters output.

Addresses #1089, supersedes #1099, enables #765

BREAKING CHANGE: Instead of --level use --loglevel. The option is no longer able to affect reporter output, it only affects application logging output. Use --loglevel=silent instead of --silent, which is now removed. The --loglevel option now only accepts 'silent', 'error', 'warning', 'debug' as values.
honzajavorek added a commit that referenced this issue Jan 24, 2019
A part of the effort to separate application logging from the reporters output.

Addresses #1089, supersedes #1099, enables #765

BREAKING CHANGE: Instead of --level use --loglevel. The option is no longer able to affect reporter output, it only affects application logging output. Use --loglevel=silent instead of --silent, which is now removed. The --loglevel option now only accepts 'silent', 'error', 'warning', 'debug' as values.
honzajavorek added a commit that referenced this issue Feb 1, 2019
A part of the effort to separate application logging from the reporters output.

Enables #1089 and subsequently #765, supersedes #1099

BREAKING CHANGE: The --level option is no longer able to affect reporter output, it only affects application logging output.
honzajavorek added a commit that referenced this issue Feb 1, 2019
A part of the effort to separate application logging from the reporters output.

Addresses #1089, supersedes #1099, enables #765

BREAKING CHANGE: Instead of --level use --loglevel. The option is no longer able to affect reporter output, it only affects application logging output. Use --loglevel=silent instead of --silent, which is now removed. The --loglevel option now only accepts 'silent', 'error', 'warning', 'debug' as values.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Epic: CLI reporter Dredd's default reporter
Projects
None yet
Development

No branches or pull requests

3 participants