这是indexloc提供的服务,不要输入任何密码
Skip to content

Output enhancements #1

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

Merged
merged 7 commits into from
Jan 22, 2015
Merged

Output enhancements #1

merged 7 commits into from
Jan 22, 2015

Conversation

nowells
Copy link
Collaborator

@nowells nowells commented Jan 22, 2015

Hey, great library! We previously used tap-spec, but then I discovered this library and was ecstatic, however, I missed a few niceties from tap-spec, I thought I would throw them in here to see if they appealed to you as well.

@namuol
Copy link
Owner

namuol commented Jan 22, 2015

Could you provide an example of how/when this affects output?

I'm just having trouble seeing differences between the tests of this and the master branch; a screenshot would be great. :)

Thanks!

@nowells
Copy link
Collaborator Author

nowells commented Jan 22, 2015

@namuol sure thing! It just adds more breathing room to results.

1__bash

@namuol
Copy link
Owner

namuol commented Jan 22, 2015

I'd rather not output the name of every successful test (i.e. should be equal), since I almost never want to read those unless there's a specific test failing, and I never actually use meaningful test names for individual tests; I just use them for describing groups of tests (i.e. the JSON output of the response should match the input).

The checkmarks keep the output "active" while tests are going on.

I agree this looks better for the trivial case from the test script, but in practice all that extra indentation is bound to increase the likelihood of word-wrapping.

What's the most important change from this PR, as far as your team is concerned?

@nowells
Copy link
Collaborator Author

nowells commented Jan 22, 2015

What's the most important change from this PR, as far as your team is concerned?

Well we explicitly name each assertion i.e. t.equal(servers.length, 5, 'ensure the server list length is correct'); so it is very helpful to see each assertion output, but sounds like that should be a configurable thing since not everyone uses assertion naming. Want me to give a step at adding a CLI arg to enable this verbose mode?

@namuol
Copy link
Owner

namuol commented Jan 22, 2015

Nah, it's generally a good practice to name your asserts. I'd rather this remain an "opinionated" output formatter to keep maintenance simple. Otherwise, everyone will want their own options...

I'll merge your changes in; the more I think about it, the more it makes sense to show that stuff.

namuol added a commit that referenced this pull request Jan 22, 2015
@namuol namuol merged commit 8f69cce into namuol:master Jan 22, 2015
@nowells
Copy link
Collaborator Author

nowells commented Jan 22, 2015

@namuol OK, I added a --verbose flag. Here is the output:

Without --verbose

1__bash

With --verbose

1__bash

@nowells
Copy link
Collaborator Author

nowells commented Jan 22, 2015

@namuol thanks! I did just push up a commit to add the --verbose flag, if you want to see it come in as a --progress flag instead to allow for minimal output I'm happy to re-work.

@nowells
Copy link
Collaborator Author

nowells commented Jan 22, 2015

@namuol nowells@fc245f9 is the commit that as the current --verbose flag the produces the above output.

@namuol
Copy link
Owner

namuol commented Jan 22, 2015

The line-numbers were a nice touch.

But really, I'd just like it to remain opinionated.

That is to say, the fewer CLI options, the better. I'm thinking it should be verbose for errors, and otherwise just show the names of every successful test. Essentially what --verbose already does...

@namuol
Copy link
Owner

namuol commented Jan 22, 2015

Seeing as you're probably more invested in this package than I am at the moment, would you mind if I made you a collaborator on the project so you can commit/npm publish at will?

@nowells
Copy link
Collaborator Author

nowells commented Jan 22, 2015

@namuol awesome! Thanks! That would be wonderful. I am nowell@strite.org on npm.

@namuol
Copy link
Owner

namuol commented Jan 22, 2015

Done! I'll add you to NPM as well so you can do npm publish whenever you version-bump.

🍻

@namuol
Copy link
Owner

namuol commented Jan 22, 2015

Also, I'll add you to tap-parser-yaml since these two were sorta forked in tandem.

@nowells
Copy link
Collaborator Author

nowells commented Jan 22, 2015

@namuol awesome. Are you going to take care of version bumping and publishing this particular change? Or want this to my first publish to this package?

@nowells
Copy link
Collaborator Author

nowells commented Jan 22, 2015

@namuol looks like you already did it. Thanks! We will be integrating this into our test chain, so I will definitely start working on any bugs that we find. Thanks again for a great library!

@namuol
Copy link
Owner

namuol commented Jan 22, 2015

Yup, I took care of version bumps for both libs tonight, but for any future changes that you integrate into your workflow, feel free to do the bumps on github and NPM. Just stick to the basic rules of semantic versioning and all that jazz. 👍

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

Successfully merging this pull request may close these issues.

3 participants