Skip to content
This repository has been archived by the owner on Sep 18, 2019. It is now read-only.

Testing Rules #24

Closed
arb opened this issue Sep 22, 2014 · 13 comments
Closed

Testing Rules #24

arb opened this issue Sep 22, 2014 · 13 comments

Comments

@arb
Copy link

arb commented Sep 22, 2014

Create a document that lays out the "rules" for lab test layouts for the hapijs universe. The current proposal is:

[...] function names but instead of '#name' use 'name()'
any non-func specific tests should be in the top describe
and groups should use lower case titles (e.g. 'vision' + 'multiple engines' + 'returns error on..')
testing for classes should use the class name in first char uppercase ('Response')
generic integration tests should just be in the top outside a describe

Thoughts? Concerns? Clarifications?

@nlf
Copy link

nlf commented Sep 22, 2014

When you say 'function names' is the assumption that test modules are a 1:1 correlation with library modules? As in, if lib/request.js exists there should be a matching test/request.js module that tests each exported method and/or class?

@arb
Copy link
Author

arb commented Sep 22, 2014

So like in hoek for example, instead of describe('#clone') it would be describe('clone()') instead.

@nlf
Copy link

nlf commented Sep 22, 2014

Right, I was getting more at naming and splitting up of test modules.

@arb
Copy link
Author

arb commented Sep 22, 2014

Ah. Yea I believe that is in the spirit of what we are trying to do.

@nlf
Copy link

nlf commented Sep 22, 2014

I thought so, definitely worth documenting though so we can be sure it's consistent everywhere

@arb
Copy link
Author

arb commented Sep 22, 2014

Yeah I think the flow is going to be discuss it here, and once it's nailed down, have a wordsmith write something up over in /contrib

@hueniverse
Copy link

@nlf if you are describing a class method, there should be one describe('Name') and inside describe('method()').

@nlf
Copy link

nlf commented Sep 22, 2014

@hueniverse right, but in that case you would have lib/name.js which is the module that exports the class and test/name.js which contains the tests for the class is the assumption I was trying to clarify.

@hueniverse
Copy link

I like to keep test files mapped to the source files. Half our modules have a single index.js.

@jardakotesovec
Copy link

Could someone elaborate bit more on rule 3? Does outside of any describe basically mean inside the top describe? Like inside describe('Mongo',... in case of catbox-mongodb? Or I misunderstood completely this rule and it refers to something else.

In any case I struggle to find clear line between what should go inside methods sections and what should go to class section.

Here is the example from catbox-mongodb that I don't understand:
This test is in class section:

    it('gets an item after settig it', function (done) {

        var client = new Catbox.Client(Mongo);
        client.start(function (err) {

            var key = { id: 'x', segment: 'test' };
            client.set(key, '123', 500, function (err) {

                expect(err).to.not.exist;
                client.get(key, function (err, result) {

                    expect(err).to.equal(null);
                    expect(result.item).to.equal('123');
                    done();
                });
            });
        });
    });

and this is in get() method section:

        it('is able to retrieve an object thats stored when connection is started', function (done) {

            var options = {
                partition: 'unit-testing',
                host: '127.0.0.1',
                port: 27017,
                poolSize: 5
            };
            var key = {
                id: 'test',
                segment: 'test'
            };

            var mongo = new Mongo(options);

            mongo.start(function () {

                mongo.set(key, 'myvalue', 200, function (err) {

                    expect(err).to.not.exist;
                    mongo.get(key, function (err, result) {

                        expect(err).to.not.exist;
                        expect(result.item).to.equal('myvalue');
                        done();
                    });
                });
            });
        });

These two tests seems to have identical (or almost identical) scope..

@jardakotesovec
Copy link

Does my question make sense? In that particular example in my previous post I don't understand if its intention to have the same test basically twice there or just oversight.

And still I would like to hear what decision process you have when you organize tests. In trivial cases its clear. For example when it just test that particular function, it is obvious why its in method section like this:

        it('passes an error to the callback when the connection is closed', function (done) {

            var options = {
                partition: 'unit-testing',
                host: '127.0.0.1',
                port: 27017,
                poolSize: 5
            };
            var mongo = new Mongo(options);

            mongo.get('test', function (err) {

                expect(err).to.exist;
                expect(err).to.be.instanceOf(Error);
                expect(err.message).to.equal('Connection not started');
                done();
            });
        });

But it is not clear when you for example test both get and set in one test, because they work together. In such case I would put it to class section. I am pretty new to writing these tests and just want to make sure that I follow hapi guidelines when creating new tests.

@hueniverse
Copy link

@arb Can you address the questions above?

@arb
Copy link
Author

arb commented Nov 2, 2014

I think the rule means that basic integration tests (like "it works") don't need to be nested inside an outer describe block. It's to cut down on superfluous describe blocks. I think testing rules might be one of those things where it'll be difficult, if not impossible to have a rule for every conceivable situation and it will fall on the test writer to decide what is best.

Also, there was conversation about lab auto generating the outer level describe, it doesn't yet, so that could be adding to the confusion as well. I've open an issue on lab to address this.

As for testing two things in one test... you generally should avoid doing that. Each test should really be testing one thing. So in your get and set example, while each test may have very similar code, you would want two tests to test each method separately.

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

No branches or pull requests

4 participants