fuzzy-ai / perjury

False vows
Apache License 2.0
3 stars 2 forks source link

Implicitly create "it works" tests #26

Closed strugee closed 6 years ago

strugee commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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

evanp commented 6 years ago

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