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

Simplify by using mocha's tap reporter #4

Merged
merged 5 commits into from
Jun 14, 2014

Conversation

jb55
Copy link
Contributor

@jb55 jb55 commented Jun 12, 2014

Advantages:

  • Smaller, simpler code
  • Human readable by default
  • Easy to pipe into other tap tools and reporters like tap-{spec,nyan} etc

Here's what it looks like now:

$ mocha-broken
1..2
not ok 1 GET /track/dl/:id 200 on proper req
  Error: expected 500 "Internal Server Error", got 302 "Moved Temporarily"

not ok 2 something timed out
  Error: timeout of 2000ms exceeded
    at [object Object].<anonymous> (/Users/jb55/dev/mocha-broken/node_modules/mocha/lib/runnable.js:139:19)
    at Timer.listOnTimeout [as ontimeout] (timers.js:110:15)

# tests 2
# pass 0
# fail 2

Piping to a reporter:

$ mocha-broken | tap-spec

Let me know what you think

jb55 added 4 commits June 12, 2014 12:45
Advantages:

  * Smaller, simpler code
  * Human readable by default
  * Easy to pipe into tap reporters like tap-{spec,nyan,etc}
@juliangruber
Copy link
Contributor

With all the mocha reporters integrated, output will also be human readable by default. Why didn't the mocha-reporter (can't remember the name) approach work? And iirc tap didn't have all the information mocha reporters need.

@jb55
Copy link
Contributor Author

jb55 commented Jun 13, 2014

Felt cleaner to me because:

  • json-stream provides less info than tap
  • json-stream lacks errors and suites, tap only lacks suites.
  • For mocha-broken, missing suites isn't too big a deal from my test cases.
  • There are more tap reporters that just worked out of the box
  • It is now human readable by default instead of json output or an opinionated default
  • A custom mocha intermediate format that isn't well defined can make it hard to support all mocha reporters (I was experiencing random issues with different reporters)

I feel this is a pretty elegant until there is a well defined intermediate format for mocha runs.

@juliangruber
Copy link
Contributor

json-stream provides less info than tap
json-stream lacks errors and suites, tap only lacks suites.

Is there a format that has all the info?

For mocha-broken, missing suites isn't too big a deal from my test cases.
There are more tap reporters that just worked out of the box
It is now human readable by default instead of json output or an opinionated default

I'd make the default the dot reporter, just as mocha has, once we have reporters merged in.

A custom mocha intermediate format that isn't well defined can make it hard to support all mocha reporters (I was experiencing random issues with different reporters)

Hmmm yeah I'll merge this for now until we have a proper mocha solution

}
var tap = parser();

ps.stdout.on('data', function(line){ process.stdout.write(line.toString()) })
Copy link
Contributor

Choose a reason for hiding this comment

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

semicolons

@jb55
Copy link
Contributor Author

jb55 commented Jun 13, 2014

Is there a format that has all the info?

I don't think so. I don't think it would be too hard though.

Hmmm yeah I'll merge this for now until we have a proper mocha solution

Yeah I just wanted something quick that wasn't ugly. An updated json-stream reporter combined with mocha-report would be nice eventually to get the full suite output.

semicolons

Hmm I used to have this linted with syntastic, not sure what happened. Will update...

@jb55
Copy link
Contributor Author

jb55 commented Jun 13, 2014

👍 linted

juliangruber added a commit that referenced this pull request Jun 14, 2014
Simplify by using mocha's tap reporter
@juliangruber juliangruber merged commit 637fac1 into segment-boneyard:master Jun 14, 2014
@juliangruber
Copy link
Contributor

sweet, released as 1.0.0

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

Successfully merging this pull request may close these issues.

None yet

2 participants