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

Implement RFC 232 (New QUnit Testing API) #208

Closed
rwjblue opened this issue Nov 10, 2015 · 25 comments
Closed

Implement RFC 232 (New QUnit Testing API) #208

rwjblue opened this issue Nov 10, 2015 · 25 comments

Comments

@rwjblue
Copy link
Member

rwjblue commented Nov 10, 2015

I would like to support nested modules, but I am unsure of the exact API.

Current front runner:

import { moduleFor, test } from 'ember-qunit';
import { module } from 'qunit';

moduleFor('foo:bar', function(hooks) {
  hooks.beforeEach(function(assert) {
    // any before each code needed
  });

  test('some non-nested test', function(assert) {
    // stuff here
  });

  module('additional grouping', function(hooks) {
    hooks.beforeEach(function(assert) {
      // setup that is specific to `additional grouping` module
    });

    test('some test name here', function(assert) {
      // stuff
    });
  });
});

Random thoughts:

  • I'm not terribly happy with the above example having to import module from qunit and moduleFor from ember-qunit. I suppose I could expose module from ember-qunit, but this is something that we have avoided so far....
  • I'd like to forbid nested moduleFor invocations. If you are testing two different modules, IMHO, you should be using different test files. I am uncertain how you could stack two moduleFor calls anyways (because both of their setups would try to "own" the test context and whatnot). Need to determine how to detect that we are being called in a nested context...
@toddjordan
Copy link

I think it would make sense to import it from ember-qunit, since you are already importing the test function from there anyway.

Agree with forbidding nested moduleFor.

Would calls to this work only within the module? I'm thinking about component integration tests where you set up variables for the template using the context.

@rwjblue
Copy link
Member Author

rwjblue commented Nov 10, 2015

Would calls to this work only within the module?

this should be the right thing in tests directly under the moduleFor or in the nested modules (I added a test in the example above directly inside the moduleFor).

@rwjblue
Copy link
Member Author

rwjblue commented Nov 10, 2015

I think it would make sense to import it from ember-qunit, since you are already importing the test function from there anyway.

Ya, seems a bit like a troll the way I initially proposed it.

@leobalter
Copy link

I'm not terribly happy with the above example having to import module from qunit and moduleFor from ember-qunit. I suppose I could expose module from ember-qunit, but this is something that we have avoided so far....

@rwjblue, is the problem on exposing module due to the name? If the nested modules are really necessary for ember-qunit they could be exposed with a different name, like suite.

I'd like to forbid nested moduleFor invocations. If you are testing two different modules, IMHO, you should be using different test files. I am uncertain how you could stack two moduleFor calls anyways (because both of their setups would try to "own" the test context and whatnot). Need to determine how to detect that we are being called in a nested context...

That's a very reasonable PoV. Keep in mind the advantage (maybe the only) on nested modules is sharing module hooks and this. I don't see how this could be useful on an ember module/component.

@leobalter
Copy link

anyway, ping me if you want any help from QUnit or even to give a feedback.

@notmessenger
Copy link

@leobalter We have the same few test cases we have in every component integration test that we consistently name across all of our components. These test case are in addition to other ones that are specific to each respective component. I anticipate using nested models as a way to group these "default" test cases. Admittedly, I have not done enough research yet to ascertain the viability of this.

@rwjblue
Copy link
Member Author

rwjblue commented Nov 11, 2015

is the problem on exposing module due to the name? If the nested modules are really necessary for ember-qunit they could be exposed with a different name, like suite.

No, mostly that the example in the issue description had some things imported from ember-qunit and other things imported from qunit. I think that would lead to pretty big confusion for our users. I'll likely do as @toddjordan mentioned, and just allow import { moduleFor, module, test } from 'ember-qunit';.

Keep in mind the advantage (maybe the only) on nested modules is sharing module hooks and this. I don't see how this could be useful on an ember module/component.

Ya, exactly. The kind of sharing and grouping that folks could do with:

moduleForComponent('asdfasd', function() {
  module('hjkl', function() {
    test('some thing', function(assert) { });
  });
});

Is perfect, but to have:

moduleForComponent('asdf', function() {
  moduleForComponent('hjkl', function() {
    test('some thing', function(assert) { });
  });
});

Would be pretty confusing since it is completely non-obvious what the behavior should actually be. ember-qunit has a whole slew of default setup steps that happen normally for moduleForComponent (many of which set things on the test context) and having one inside another would be extremely confusing.

ping me if you want any help from QUnit or even to give a feedback.

Thanks @leobalter! Will keep you updated as we progress...

@rwjblue
Copy link
Member Author

rwjblue commented Nov 11, 2015

I anticipate using nested models as a way to group these "default" test cases

Completely agreed, grouping (for shared beforeEach/afterEach logic and naming) is a huge part of why I am excited about nested modules.

@toddjordan
Copy link

@rwjblue What I meant with my question about this is the this object the same instance between the moduleFor and nested modules, and if so, would the the setup within a module be cleaned? My guess is it would be the same instance and that you would have to handle tear down of state, which is fine. I guess if it were different. you could set up your test context separately between modules, kinda like below, without worrying about module state leaking into the next module.

moduleForComponent('mycomp', function() {
  module('when state is x', function(hooks) {
    hooks.beforeEach(function() {
      this.set('myFn', myStubFnA);
    };
    test('should do j', function(assert) {
      ...
    }; 
    test('should do k', function(assert) {
      ...
    }
  };
  module('when y', function(hooks) {
    test('should do l', function(assert) {
      ...
    }; 
  };
}

@toddjordan
Copy link

also is it possible not to have to deal with the hooks object? In doing some jasmine in the past, modules were handled with a describe function and tests with an it function, and you can call them in a nested fashion without having to pass in an object to invoke them on

@rwjblue
Copy link
Member Author

rwjblue commented Nov 11, 2015

@toddjordan - Each test gets its own test context which is made this inside the test and all of its setup (just like what happens today). All beforeEach hooks run in order (from outside in) and the this context in all of them is the same in all of them (so they combine).

In your example:

  • should do j - Gets its own this, runs the normal moduleForComponent setup stuff, runs where state is x beforeEach, then runs the test itself.
  • should do k - Same as above.
  • should di l - Gets its own this, runs the normal moduleForComponent setup stuff, runs when y beforeEach (which doesn't exist in your demo), then runs the test itself.

also is it possible not to have to deal with the hooks object?

Not really (as far as I know, but @leobalter might have additional insight). For ember-qunit itself, I'd like to stay as close to QUnit's nested module implementation as possible and not invent our own syntax any more than we have too...

In doing some jasmine in the past, modules were handled with a describe function and tests with an it function, and you can call them in a nested fashion without having to pass in an object to invoke them on

You are basically describing mocha, and there is an ember-mocha project for that 😸

@toddjordan
Copy link

Thanks, makes sense. one more question... would tests at the moduleFor level be defined within the moduleFor function? Currently the are defined outside, but it might make more sense like below with nested modules:

moduleForComponent('x', function() {
  test('should do x', function(assert) {
  };
  module('when doing something', function() {
    ...
  };
};

@rwjblue
Copy link
Member Author

rwjblue commented Nov 11, 2015

would tests at the moduleFor level be defined within the moduleFor function?

Yes. See my example in the issue description, it looks roughly like what you have here.

@toddjordan
Copy link

👍 from me, I'm looking forward to the new capability 😎

@leobalter
Copy link

also is it possible not to have to deal with the hooks object?

Not really (as far as I know, but @leobalter might have additional insight). For ember-qunit itself, I'd like to stay as close to QUnit's nested module implementation as possible and not invent our own syntax any more than we have too...

The hooks object on modules are optional, you may use them as the second argument on a module call or from the first parameter of the callback function.

// hooks as arguments, they look messy with the nested function:
module('my module',
  {
    beforeEach: function(assert) {
      assert.ok(true);
    }
  },
  function() {
    test('foo', ...);
  }
);
// hooks from the callback parameter, they are functions here
module('my module', function(hooks) {
  hooks.beforeEach(function(assert) {
    assert.ok(true);
  });

  test('foo', ...);
);

// if you have a ES2015 ready environment:
module('my module', function({beforeEach}) {
  beforeEach(function(assert) {
    assert.ok(true);
  });

  test('foo', ...);
);

// If you really like describe and it methods:
// if you have a ES2015 ready environment:
var {
  describe: module,
  it: test
} = QUnit; 
describe('my module', function({beforeEach}) {
  beforeEach(function(assert) {
    assert.ok(true);
  });

  it('foo', ...);
);

You can write your modules without using any hook:

module('my module', function() {
  test('foo', ...);
});

@phcoliveira
Copy link

I am happy to see that this functionality, which is already provided by QUnit, will be shipped to Ember-QUnit.

I am struggling with test names ever since I moved to Ember, it feels weird to do BDD without nested modules, and it also feels weird to do unit testing for a product. I guess that you guys, when making the framework, don't quite feel the need for testing behaviour, but I certainly feel it when making a web application.

@trek
Copy link
Member

trek commented Jan 10, 2016

@rwjblue

Only chiming into mostly echo since you asked:

I think your current front runner is what most people would expect.

I totally see why nested moduleFor would not makes sense. It's analogous to rspec's

describe AClass, type: :some_type do
end

And declaring a new described_class with a different type within a describe call like that... I don't know what it would even mean. It would be complicated from a code perspective and it would be confusing to a reader later on.

The only bikeshedding I have to offer is that the name hooks feels odd. I've been using config as a variable name, but this probably doesn't matter if ember-cli generates files using @leobalter ES2015 example style.

@rwjblue
Copy link
Member Author

rwjblue commented Jul 5, 2016

@trentmwillis - We should circle back around on this one...

@jeremy-w
Copy link

jeremy-w commented Jul 5, 2016

  • Barring nesting moduleFors makes plenty of sense.
  • Would it make sense to re-export QUnit.module as submodule, in line with the expected use as subordinate to a moduleFor-created module?
  • Would non-nested uses as done today continue to work without change, where the module decl sets the module for all following tests in the file?
  • FWIW, the nesting style I expected was a moduleFor with 3 args: name, the current second object arg for hooka, and an optional third of a function that can define tests and submodules within. This is what I saw in QUnit's nesting usage, but it differs from the two-arg version shown in the issue, where the hooks are instead installed on an argument to the "module body" function. But if there are docs or templates showing how it's done, I don't think the difference will at all be any issue in practice.

@rwjblue
Copy link
Member Author

rwjblue commented Jul 5, 2016

Would it make sense to re-export QUnit.module as submodule, in line with the expected use as subordinate to a moduleFor-created module?

If you mean import { module } from 'ember-qunit'; then yes I think we should allow that.

Would non-nested uses as done today continue to work without change, where the module decl sets the module for all following tests in the file?

Yes, this is default QUnit behavior and must continue to work (for backwards compat).

FWIW, the nesting style I expected was a moduleFor with 3 args: name, the current second object arg for hooks, and an optional third of a function that can define tests and submodules within. This is what I saw in QUnit's nesting usage, but it differs from the two-arg version shown in the issue, where the hooks are instead installed on an argument to the "module body" function.

Both versions are supported and documented under QUnit.module, and I think we would want to support both here also.

@jeremy-w
Copy link

jeremy-w commented Jul 6, 2016

Sounds good. What I meant with the submodule question was whether to reexport QUnit.module as a function named submodule from ember-qunit, because of its subordinate role in that module's context.

@trentmwillis
Copy link
Member

I'm onboard with this. I'd like to start pushing us in a direction where we nest modules more anyway as it allows for better organization/filtering.

I think the implementation will be fairly straightforward so long as we don't allow moduleFor calls to be nested (we should probably throw an error if it is attempted). I'm not super familiar with the code here, but I think proxying the arguments to support the various signatures shouldn't be too difficult.

What I meant with the submodule question was whether to reexport QUnit.module as a function named submodule from ember-qunit

I think we should avoid renaming/aliasing functions, but we can make our examples more explicit by giving examples like:

import { moduleFor, module as subModule } from 'ember-qunit';

@jbescoyez
Copy link

I see this is on hold for more than 1 year. Is there a workaround that allows to nest Qunit modules?

@rwjblue
Copy link
Member Author

rwjblue commented Oct 9, 2017

Yes, work is underway to implement emberjs/rfcs#232 which removes all of our custom wrapping around QUnit (and therefore allows us to fully embrace QUnit’s functionality here).

@Turbo87 Turbo87 changed the title Add support for nested modules when on QUnit 1.20. Implement RFC 232 (New QUnit Testing API) Oct 14, 2017
@rwjblue
Copy link
Member Author

rwjblue commented Oct 17, 2017

Closed by #286.

@rwjblue rwjblue closed this as completed Oct 17, 2017
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

10 participants