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

ENHANCEMENT - setupPageComponentTest for hooks with easier setup with test APIs #444

Closed
rtablada opened this issue Feb 6, 2019 · 5 comments · Fixed by #448
Closed

ENHANCEMENT - setupPageComponentTest for hooks with easier setup with test APIs #444

rtablada opened this issue Feb 6, 2019 · 5 comments · Fixed by #448
Labels

Comments

@rtablada
Copy link
Contributor

rtablada commented Feb 6, 2019

Just a suggestion in our app we have started using a hook helper function (in QUnit tests) like this:

export function setupPageComponentTest (hooks, page) {
  hooks.beforeEach(function() {
    page.setContext(this);
  });

  hooks.afterEach(() => {
    page.removeContext();
  });
}

This removes a lot of boilerplate from our component tests since we don't need to set the context and remove context for component tests using EPO.

I'm not sure if this is something other teams would value?

The one worry I have (and why I did not create a PR yet) is that this helper is coupled to ember-qunit...

@san650
Copy link
Owner

san650 commented Feb 7, 2019

I think it makes sense to include it in the package. Can you create a PR?

@rtablada
Copy link
Contributor Author

rtablada commented Feb 7, 2019

Yeah I'll make a PR and some docs on how to use ECPO in integration tests for components.

@ro0gr
Copy link
Collaborator

ro0gr commented Feb 7, 2019

Hi, there is actually a PR already for that #432

@ro0gr
Copy link
Collaborator

ro0gr commented Feb 7, 2019

As far as I understand this helper is supposed to enable moduleForComponent mode for the page object. Is it correct or do you need it in the setupRenderingTest for some reason?

I think a test helper for a specific page object is a bad path to go. What if the test needs few page objects to use? The relevant test setup would look something like:

  setupPageComponentTest(hooks, page1);
  setupPageComponentTest(hooks, page2);

which doesn't look good to me, since we never want different test modes within the same test.

I mean that the test mode seems like a global state within a test to me. But we currently store and read it from the specific pageObject instance. I think with the new test helper we can do better, by setting a test mode globally and then take care to clean it up in afterEach, so we don't need to pass a page object instance to it:

  setupPageObjects(hooks, {
    moduleForComponent: true
  })

I think it's even possible to do it in a backward compat way(supporting setContext as a fallback)

Hope it makes sense and sorry for quite confusing thoughts dump(a bit under preassure now)...

@bendemboski
Copy link
Contributor

My 2 cents: #432 (comment)

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

Successfully merging a pull request may close this issue.

4 participants