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

Acceptance test tentative API #38

Merged
merged 8 commits into from Mar 23, 2017
Merged

Acceptance test tentative API #38

merged 8 commits into from Mar 23, 2017

Conversation

cibernox
Copy link
Owner

This PR is opened with a failing test that demonstrates the tentative API
for acceptance test, including an example of asserting transient loading
substates.

@cibernox
Copy link
Owner Author

@machty You might also be interested.

Copy link
Collaborator

@pixelhandler pixelhandler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cibernox nice to see the beginning of using native helpers for acceptance as well :) I had a couple comments here as well as on the issue #24

@@ -1,7 +1,7 @@
module.exports = {
root: true,
parserOptions: {
ecmaVersion: 6,
ecmaVersion: 2017,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this 👍


moduleForAcceptance('Acceptance | usage in acceptance');

test('Usage awaiting the world to settle', async function(assert) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the use of async / await require changes in the build, e.g. babel config?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, babel 6 will know what to transpile automatically based on the browsers you support.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me amend that thing above.
In order for async/await to work you have to have the regenerator runtime (via ember-maybe-import-regenerator), include the babel polyfill or use the new targets feature (https://github.com/ember-cli/rfcs/blob/master/complete/0095-standardise-targets.md) in ember-cli 2.13 and only support browsers with async/await.

@@ -1,20 +1,20 @@
<h1>Signup example</h1>

<form {{action "signup" on="submit"}}>
<form class="signup-example-form" {{action "signup" on="submit"}}>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have an example form component used for component tests. Would it be feasible to combine the two use cases, e.g. test a form component via integration test and use the same form component in the app for an acceptance test?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if I follow. The helpers will be the same, with the exception that some of them, like visit should probably raise an error if used in integration

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see. I didn't consider it. I wanted to create an example with async behaviour (loading substate, disable a button while a request is ongoing...) that is hard to test currently to prove that this approach would allow it.

I think that we can keep them separated.

@@ -0,0 +1,33 @@
import { test } from 'qunit';
import moduleForAcceptance from '../../tests/helpers/module-for-acceptance';
import { visit, click, fillIn, waitUntil } from 'ember-native-dom-helpers/test-support/helpers';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to do anything special as we're masking the global helpers here?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach masks the global helpers, although they would be reachable using window

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I know we're masking intentionally but I generally think it's not the best example to show off something. I get that we may need to do that. I wonder if a comment that says "We're masking the global helpers here, they are still available, e.g use window.find"

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that should be documented, although my intention with this PR is bring attention over how this syntax and approach feels.
@machty p.e suggested making find/findAll async, and accept modifiers like find('.some-class', { count: 5 }). I then suggested the existence of the waitUntil helper and keep finders sync. That kind of feedback.

@rwjblue
Copy link
Collaborator

rwjblue commented Mar 23, 2017

@cibernox - Looks good. I think the initial API for visit is pretty simple:

export default function visit() {
  if (!window.visit) {
    throw new Error('visit is only available during acceptance tests');
  }

  return window.visit(...arguments);
}

This PR is opened with a failing test that demonstrates the tentative API
for acceptance test, including an example of asserting transient loading
substates.
@cibernox
Copy link
Owner Author

@rwjblue I've implemented the waitUntil helper.

The acceptance test that I wrote initially now is green (yay TDD!) and I added tests for the helper in integration.
Now this should be green.

@rwjblue rwjblue merged commit c1b6f90 into master Mar 23, 2017
@rwjblue rwjblue deleted the make-it-work-in-acceptance branch March 23, 2017 19:04
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

Successfully merging this pull request may close these issues.

None yet

3 participants