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

Allow nested suites (modules) #543

Closed
JamesMGreene opened this issue Mar 12, 2014 · 15 comments
Closed

Allow nested suites (modules) #543

JamesMGreene opened this issue Mar 12, 2014 · 15 comments
Assignees
Labels
Category: API Type: Enhancement New idea or feature request. Type: Meta Seek input from maintainers and contributors.
Milestone

Comments

@JamesMGreene
Copy link
Member

A personal sore point for me when using QUnit is the lack of nested suites (modules) that are offered by other popular frameworks like Mocha and Jasmine.

I've given it some cursory thought and, if we agree it adds value, I don't foresee any major roadblocks to implementing that capability. It would give us some options for richer reporting outputs as well, e.g. Mocha's sleek nested HTML report:

screen shot 2014-03-12 at 1 00 06 pm

@JamesMGreene JamesMGreene added this to the v2.0 milestone Mar 12, 2014
@dcherman
Copy link

+1000!

The lack of nested modules ( like nested describes ) in QUnit have been one of my major drivers for using mocha in newer projects.

Even better if it supports setup/teardown within those nested modules so we can get the equivalent of beforeEach and afterEach

@JamesMGreene
Copy link
Member Author

This could also work wonders to cleaning up the HTML report for the jquery/qunit-composite runner.

@jzaefferer
Copy link
Member

What API would you use for this?

@Krinkle
Copy link
Member

Krinkle commented Apr 8, 2014

API idea:

QUnit.test( 'APP.Foo', {
  setup: fn,
  teardown: fn
}, function( assert ) {
  var foo = new App.Foo();
  assert.ok( foo, 'instantiation' );

  assert.test( 'foo.add', {
    setup: function () {
      foo = new App.Foo();
    }
  }, function( assert ) {
    assert.test( 'foo.add( String )', function( assert ) {
      assert.equal( foo.add( 'x' ), 'default x' );
      assert.equal( foo.add( 'y' ), 'default x y' );
    } );
    assert.test( 'foo.add( String... )', function( assert ) {
      assert.equal( foo.add( 'x', 'y' ), 'default x y' );
      assert.equal( foo.add( 'a', 'b', 'c' ), 'default x y a b c' );
    } );
  } );

  assert.test( 'foo.reset()', function( assert ) {
    foo.add( 'x', 'y' );
    assert.equal( foo.reset(), 'default' );
  } );

} );


QUnit.test( 'APP.Bar' );
...

Which would be more-or-less the equivalent of:

QUnit.module( 'APP.Foo', {
  setup: fn,
  teardown: fn
} );

(function() {
  var foo = new App.Foo();
  QUnit.test( 'foo.add', function( assert ) {
    assert.ok( foo );
  } );

  QUnit.test( 'foo.add( String )', function( assert ) {
    foo = new App.Foo();
    assert.equal( foo.add( 'x' ), 'default x' );
    assert.equal( foo.add( 'y' ), 'default x y' );
  } );

  QUnit.test( 'foo.add( String... )', function( assert ) {
    foo = new App.Foo();
    assert.equal( foo.add( 'x', 'y' ), 'default x y' );
    assert.equal( foo.add( 'a', 'b', 'c' ), 'default x y a b c' );
  } );

  QUnit.test( 'foo.reset()', function( assert ) {
    foo = new App.Foo();
    foo.add( 'x', 'y' );
    assert.equal( foo.reset(), 'default' );
  } );
}());

QUnit.module( 'APP.Bar' );
...

Right now we have special "module" groups that can only contain tests and supports setup/teardown,
and another special group type called "test" that can only contain assertions. Instead, we'd only have
"test" groups which can contain both assertions and/or other tests, and with support for setup/teardown.

  groups:
    - APP.Foo
      groups:
      - foo.add
        assertions: [ok]
        groups:
        - foo.add( String )
          assertions: [equal, equal]
        - foo.add( String... )
          assertoins: [equal, equal]
      - foo.reset
        assertions: [equal]
    - APP.Bar
      ...

@dcherman
Copy link

dcherman commented Apr 8, 2014

Another idea which is similar to other existing implementations:

QUnit.module( 'parent', function( test ) {
  // Maybe test.setupOnce() as well?
  test.setup(function() {
     // Do stuff
  });

  test.teardown(function() {
      // Do stuff
  });

  test( 'some test', function( assert ) {});
  test.async( 'an async test', function( assert ) {});

  QUnit.module( 'child', function( test ) {
      // Nested!
  });
});

@leobalter
Copy link
Member

I liked @dcherman's suggestion and I'm wondering that also would be interesting to have test level setup and teardowns, as exemplified by @Krinkle:

QUnit.test( 'APP.Foo', {
  setup: fn,
  teardown: fn
}, function() {} );

@JamesMGreene
Copy link
Member Author

I also like @dcherman's suggestion, which is very akin to Mocha's TDD style:

suite('Array', function() {

  suiteSetup(function() {
    // This runs once before any `test` within the `suite` is executed
  });

  setup(function() {
    // This runs before each `test`
  });

  teardown(function() {
    // This runs after each `test`
  });

  suiteTeardown(function() {
    // This runs once after every `test` within the `suite` has been executed
  });


  suite('#indexOf()', function() {
    test('should return -1 when not present', function() {
      assert.equal(-1, [1,2,3].indexOf(4));
    });
  });
});

@Krinkle
Copy link
Member

Krinkle commented Jul 19, 2014

This doesn't appear to be a priority at the moment. Patches are welcome, however.

Note, though, that this shouldn't affect your ability to write good and maintainable tests. It's only a minor prettiness factor (one of which the usefulness is imho still debatable). The main reason I'm in favour of it is because it'd simplify the code a lot by taking away special meaning from "module" and "test". Everything would simply be groups of assertions and/or other groups.

For setup/teardown one can use "test" right now, and for a persistent scope a closure is easy enough. I've written loads of tests over the past years and not once felt a need for this (not even with hacky closures). One QUnit module for each tangible component of the code, and one test for each logical group of assertions for an individual property, method or behaviour exposed by that component.

@Krinkle Krinkle removed this from the v2.0 milestone Jul 19, 2014
@JamesMGreene
Copy link
Member Author

I am assuming this is actually more of shift for our reporters than it is for our core module/test/assert code.

@jzaefferer
Copy link
Member

The discussion here has been mostly about nesting code, but the original ticket actually referred to the "sleek nested HTML output" of Mocha. What the screenshot shows should be possible already, without requiring any API changes. It's mostly a matter of displaying the results differently and that's something we can explore.

Otherwise, on the API/code side, I'm not inclined to change anything, for the reasons that @Krinkle has outlined above.

@jzaefferer
Copy link
Member

To clarify, the screenshot in the original ticket description shows at most two branches and one leaf node, which corresponds to our module/test/assertion methods. So achieving that specific output shouldn't require any API changes.

@JamesMGreene
Copy link
Member Author

True, though that was unintentional. Here's a different screenshot with deeper nesting:

image

My intent for this issue was to discuss the capability of test/group nesting, not [just] the HTML Reporter visuals.

@JamesMGreene
Copy link
Member Author

Added a note to #647 that, if we do implemented nested modules, we also need to rethink how our beforeEach/afterEach hooks work such that they can be handled in a nested fashion as well.

@JamesMGreene
Copy link
Member Author

If I were going to use QUnit for nested modules, I think I would want the syntax to look like this:

(function( suite, test, beforeEach ) {

  suite( "parent", function() {
    beforeEach(function( assert ) { /* ... */ });

    test( "x", function( assert ) { /* ... */ });
    test( "y", function( assert ) { /* ... */ });

    suite( "child", function() {
      beforeEach(function( assert ) {
        // also automatically invokes its parent module's `beforeEach`
        /* ... */
      });

      test( "z", function( assert ) { /* ... */ });
    });
  });

})( QUnit.module, QUnit.test, QUnit.beforeEach );

You'll note that the above example is utilizing a theoretical QUnit.beforeEach which would always bind to the current stack context. This is preferable when nesting modules as it (a) allows infinitely nested hooks with a simple API and (b) doesn't look so ugly as if you have the lifecycle/hooks object as the first param and the execution scope function as the second param.

If that API is undesirable, we could also add it to a module context object (e.g. this.beforeEach) or to an argument object passed to the module's callback (e.g. moduleContext.beforeEach).

However, I would also definitely recommend we maintain the existing syntax as well. It is simple/linear, widely used, and generally well-liked:

QUnit.module( "x", {
  beforeEach: function( assert ) { /* ... */ },
  afterEach: function( assert ) { /* ... */ }
});

QUnit.test( "y", function( assert ) { /* ... */ });

@esbanarango
Copy link

+1000!!!

leobalter pushed a commit to leobalter/qunit that referenced this issue Sep 10, 2015
Allows modules to be nested. This is achieved by adding a third possible
argument to the QUnit.module method, which is a function that is called
for immediate processing. Also, testEnvironments for parent modules
are invoked in recursively for each test (outer-first).

Fixes qunitjs#543
leobalter pushed a commit to leobalter/qunit that referenced this issue Sep 10, 2015
Allows modules to be nested. This is achieved by adding a third possible
argument to the QUnit.module method, which is a function that is called
for immediate processing. Also, testEnvironments for parent modules
are invoked in recursively for each test (outer-first).

Fixes qunitjs#543
leobalter pushed a commit to leobalter/qunit that referenced this issue Sep 10, 2015
Allows modules to be nested. This is achieved by adding a third possible
argument to the QUnit.module method, which is a function that is called
for immediate processing. Also, testEnvironments for parent modules
are invoked in recursively for each test (outer-first).

Fixes qunitjs#543
leobalter pushed a commit to leobalter/qunit that referenced this issue Sep 29, 2015
Allows modules to be nested. This is achieved by adding a third possible
argument to the QUnit.module method, which is a function that is called
for immediate processing. Also, testEnvironments for parent modules
are invoked in recursively for each test (outer-first).

Fixes qunitjs#543
leobalter pushed a commit to leobalter/qunit that referenced this issue Oct 7, 2015
Allows modules to be nested. This is achieved by adding a function
argument to the QUnit.module method, which is a function that is called
for immediate processing. Also, testEnvironments for parent modules
are invoked in recursively for each test (outer-first).

Fixes qunitjs#543
Closes qunitjs#800
Closes qunitjs#859
Closes qunitjs#670
Ref qunitjs#858
@Krinkle Krinkle changed the title Allow nested suites (modules)? Allow nested suites (modules) Jun 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: API Type: Enhancement New idea or feature request. Type: Meta Seek input from maintainers and contributors.
Development

Successfully merging a pull request may close this issue.

6 participants