testdeck / testdeck-site

the testdeck site source
https://testdeck.org
0 stars 0 forks source link

The docs should mention that a new instance of the class is created before each test #42

Open jrumbinas opened 5 years ago

jrumbinas commented 5 years ago

Testdeck Version

0.0.6

Testdeck Package

NodeJS Version

Mocha / Jasmine / Jest Version

Operating System

Actual Behaviour

Consider the following test:

import { suite, test } from "@testdeck/mocha";

[{ title: 'world'}, { title: 'github'}]
.forEach(({title}) => {

    @suite(`Hello ${title}`)
    class Hello {
        constructor() {
            console.log('');
            console.log(`Hello ${title}`);
        }

        @test
        test1() {
            console.log('test1');
        }

        @test
        test2() {
            console.log('test2');
        }
    }
});

Hello class is instantiated 4 times, i.e. once per test

Hello world
test1
Hello world
test2
Hello github
test1
Hello github
test2

Expected Behaviour

Only 2 Hello class instances should have been created

Hello world
test1
test2
Hello github
test1
test2

Additional Information

Official documentation Instance Lifecycle Hooks is providing the impression that @suite class may have an internal state, which is clearly not true.

Please note, that suite scope state can only be achieved via class instance or actual mocha suite. IMHO it's a bad practice to pollute mocha suite context.

jrumbinas commented 5 years ago

Blocks #237 [FEATURE] - Parameterized suites

silkentrance commented 5 years ago

@jrumbinas Good find. This is indeed the case. It must have slipped my mind when I made the new documentation.

The behaviour is exactly the same as the original mocha-typescript. And I vaguely remember that it was mentioned somewhere in the earlier docs.

The class instance is available in the context of the test method and so, yes, it does have an internal state. However, this will be recreated before each test and you must not rely on the instance state to just be there but use the before and after hooks to set it up.

We will add the missing information to the documentation.

jrumbinas commented 5 years ago

The class instance is available in the context of the test method and so, yes, it does have an internal state.

Agreed, let me rephrase it: the class still has internal state, however, new instance creation with each test execution effectively removes all internal state (which breaks the promise that tests are compatible between mocha and testdeck). Please see the following example where Mocha suite function is only executed once:

let mochaCounter = 0;
describe('Hello', function suiteScope() {
    const instance = ++mochaCounter;

    // Mocha could execute `suiteScope` function for each test, but it doesn't
    it('test1', function() {
        console.log("%s: instance%s\n", this.test.title, instance);
    });

    it('test2', function() {
        console.log("%s: instance%s\n", this.test.title, instance);
    });

    // Produces
    // test1: instance1
    // test2: instance1
});

let testDeckCounter = 0;
@suite
class TestDeckHello {
    private instance = ++testDeckCounter;

    @test
    test1() {
        console.log("test1: instance%s\n", this.instance);
    }

    @test
    test2() {
        console.log("test2: instance%s\n", this.instance);
    }

    // Produces
    // test1: instance1
    // test2: instance2
}

IMHO both tests should produce identical results

However, this will be recreated before each test and you must not rely on the instance state to just be there but use the before and after hooks to set it up.

I do agree it sounds like a bad practice, however, session-based API tests do take advantage of suite scope to manage some resources. Things get far more complicated once you start thinking about parametric suites.

silkentrance commented 5 years ago

@jrumbinas This is where static before and after comes into play.

Here, you can set up your static environment, e.g. have a rest client prepared and from inside the instance level before and after you can use the statics to for example sign up a new user on the site you are testing, wade through the registration process and then run your tests as normal.

So basically, the OOP interface replaces the closures from the standard TDD interface by a static scope (at the class level) and a dynamic scope (at the instance level).

And, given the nature of Javascript, statics are also inherited, so you can create rather complex scenarios there.

I will add a section about this in the documentation, too.

silkentrance commented 5 years ago

@jrumbinas I have moved this to our documentation repository since we are not going to change the the existing behaviour.

jrumbinas commented 5 years ago

This is where static before and after comes into play

I'm not saying that it cannot be achieved in other ways. Imagine the world where JS would remove all loops but simple for-loop. That's right no while, do-while, for-in, for-of and foreach(). When you think about they are really redundant and can be implemented with for-loop :)

Think about the crowd coming in from other OOP languages like Java. They will have trouble understanding such a "requirement". I'm pretty sure you won't find a mature OOP testing framework which would be re-instantiating test classes with each test method invocation.

Out of curiosity, was there any consideration which approach to use? Could you share the main arguments?

silkentrance commented 5 years ago

@jrumbinas If you look at the very first commit in the testdeck repository

https://github.com/testdeck/testdeck/commit/8ec0705e3a1f098e47c52595882d1772205b8cd8#diff-ed009b6b86b017532ef0489c77de5100R52

you will see that this design decision was made right at the beginning of the project, by then it was called mocha-typescript.

Also, since that behaviour fits our specific needs for testing, i.e. not having to care about tearing down stuff that will be added to the instance over time, e.g. dynamically added fields and so on, we kept it as it was.