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

TestModuleFor{App,Acceptance} proposal #94

Closed
seanpdoyle opened this issue Jul 31, 2015 · 7 comments
Closed

TestModuleFor{App,Acceptance} proposal #94

seanpdoyle opened this issue Jul 31, 2015 · 7 comments

Comments

@seanpdoyle
Copy link
Contributor

WRT https://github.com/rwjblue/ember-qunit/pull/71

@rwjblue @stefanpenner I'm looking into baking this into switchfly/ember-test-helpers:

// tests/helpers/module-for-acceptance.js

import Ember from 'ember';
import { module } from 'qunit';
import startApp from './start-app'; // This file is the unchanged, `ember new` version

export default function(name, options = {}) {
  let application;

  module(`Acceptance | ${name}`, {
    beforeEach: function() {
      application = startApp();
      if (options.beforeEach) {
        options.beforeEach.call(this, arguments);
      }
    },

    afterEach: function() {
      Ember.run(application, 'destroy');
      if (options.afterEach) {
        options.afterEach.call(this, arguments);
      }
    }
  });
}

So that it's integration with ember-qunit would be as minimal as the rest.

I'm having trouble figuring out how, though.

Any ideas?

Will this work at all, or is it best left as a hand-rolled helper?

@rwjblue
Copy link
Member

rwjblue commented Jul 31, 2015

I guess I don't understand what you mean.

@rwjblue
Copy link
Member

rwjblue commented Jul 31, 2015

Since we don't really need any special shenanigans to deal with bizarreness in Ember, this sort of thing likely lives in ember-qunit itself.

@seanpdoyle
Copy link
Contributor Author

Sorry if this is too cryptic.

https://github.com/rwjblue/ember-qunit/pull/71 introduces a moduleForApp to wrap

https://github.com/ember-cli/ember-cli/blob/8d9c3a4579abb65416f40ec0dd46008b3c159a49/blueprints/acceptance-test/files/tests/acceptance/__name__-test.js#L3-L13

I'd like to push that forward, but it looks like that PR predates the ember-test-helpers / ember-qunit split, so I tried to work it into this repo instead but I hit a wall.

seanpdoyle added a commit to seanpdoyle/ember-cli that referenced this issue Aug 16, 2015
seanpdoyle added a commit to seanpdoyle/ember-cli that referenced this issue Aug 16, 2015
seanpdoyle added a commit to seanpdoyle/ember-cli that referenced this issue Aug 16, 2015
seanpdoyle added a commit to seanpdoyle/ember-cli that referenced this issue Aug 27, 2015
seanpdoyle added a commit to seanpdoyle/ember-cli that referenced this issue Sep 13, 2015
seanpdoyle added a commit to seanpdoyle/ember-cli that referenced this issue Sep 17, 2015
@cowboyd
Copy link

cowboyd commented Sep 18, 2015

We've been toying around with implementing something like this for use in one of our addons. Thus far, we've been writing all the code inside the addon, but with an eye for potentially extracting it out for inclusion into ember-test-helpers

https://gist.github.com/Robdel12/c8470434a5e0ded666db

It's been working really well thus far, but my primary concern is with these lines here https://gist.github.com/Robdel12/c8470434a5e0ded666db#file-test-helper-js-L2-L3

Since this class is embedded into our addon, it has access to the (dummy) application's class and config, but is that going to pose a problem if this code were to live in ember-test-helpers?

Also, the startApp() test helper that gets generated by ember-mocha and ember-qunit acceptance-test blueprints allows passing one-off options to Application.create() per test. Changing settings suite-wide in environment.js for 'test' has always been sufficient for us, so we wouldn't miss that, but it might be a problem for others.

@cowboyd
Copy link

cowboyd commented Sep 21, 2015

Unless anybody sees anything obviously flawed with this approach, we'll go ahead and submit the PR.

@seanpdoyle
Copy link
Contributor Author

@cowboyd I say go ahead, I have an outstanding PR to ember-cli/ember-cli, but I'm having some trouble with it.

If we could get something working in this repo instead, I think we'd be better off.

seanpdoyle added a commit to seanpdoyle/ember-cli that referenced this issue Sep 25, 2015
seanpdoyle added a commit to seanpdoyle/ember-cli that referenced this issue Oct 2, 2015
homu added a commit to ember-cli/ember-cli that referenced this issue Oct 4, 2015
@seanpdoyle
Copy link
Contributor Author

Added to ember-cli directly in ember-cli/ember-cli#4692

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

No branches or pull requests

3 participants