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

Implicitly create "it works" tests #26

Closed
strugee opened this issue Jul 15, 2017 · 3 comments
Closed

Implicitly create "it works" tests #26

strugee opened this issue Jul 15, 2017 · 3 comments

Comments

@strugee
Copy link
Collaborator

strugee commented Jul 15, 2017

I find myself writing a lot of repetitive "it works" tests that do nothing but call assert.ifError(err);. Seems like it would be kind of nice if I could have Perjury just automatically construct these for me. Strawman proposal:

vows.describe('context mock object').addBatch({
	'When we require the module': {
		topic: function() {
			return require('./mocks/context');
		},
		'it\'s a factory function': function(err, makeContext) {
			assert.isFunction(makeContext);
		}
}, { handleErrors: true }).export(module);

would be equivalent to:

vows.describe('context mock object').addBatch({
	'When we require the module': {
		topic: function() {
			return require('./mocks/context');
		},
		'it works': function(err) {
			assert.ifError(err);
		},
		'it\'s a factory function': function(err, makeContext) {
			assert.isFunction(makeContext);
		}
}).export(module);

Or in other words, addBatch would take an additional options object that would allow turning this stuff on. The one reservation I have with this is that it seems to impact readability because the parameters the batch is run under are at the bottom, not the top, where they'd be more easily noticed. Perhaps we can solve this by treating the first argument as an options object if there are two arguments provided, so:

vows.describe('context mock object').addBatch({ handleErrors: true }, {
	'When we require the module': {
		topic: function() {
			return require('./mocks/context');
		},
		'it\'s a factory function': function(err, makeContext) {
			assert.isFunction(makeContext);
		}
}).export(module);

That feels kinda gross somehow, though.

I really am not sure what the best thing to do here would be.

(I also thought about doing chaining, like e.g. vows.describe('foobar').addBatch({ /* suite */ }).handleErrors() but it seems like it's less obvious that this doesn't have weird side effects and it's unclear if it applies to the single batch, the entire suite, all batches chained before/after, etc.)

@strugee
Copy link
Collaborator Author

strugee commented Jul 15, 2017

Another approach would be to not add this at all and tell users to just write their own function, but tbh this seems like such a common usecase I think it'd be better to build in support.

@strugee
Copy link
Collaborator Author

strugee commented Jul 25, 2017

As pointed out on IRC by @evanp this comes with i18n concerns. Seems like this would be a good idea to do if we have proper i18n support in the project generally? Which would be nice anyway for the reporters and stuff

@strugee strugee mentioned this issue Jul 25, 2017
Closed
@evanp
Copy link
Member

evanp commented Apr 27, 2018

I'm going to close this in favour of vowsjs/vows#386

@evanp evanp closed this as completed Apr 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants