Merge pull request #357 from bendemboski/rfc268

Support RFC268 tests

.npmignore

@@ -9,7 +9,7 @@
 .editorconfig
 .ember-cli
 .gitignore
-.jshintrc
+.eslintrc.js
 .watchmanconfig
 .travis.yml
 bower.json
@@ -20,3 +20,8 @@ Brocfile.js
 /_site
 node-tests/
 yarn.lock
+
+# ember-try
+.node_modules.ember-try/
+bower.json.ember-try
+package.json.ember-try

.travis.yml

@@ -7,21 +7,26 @@ addons:
   chrome: stable
 
 sudo: false
+dist: trusty
 
 cache:
   yarn: true
 
 env:
-  - EMBER_TRY_SCENARIO=node-tests
-  - EMBER_TRY_SCENARIO=ember-lts-2.4
-  - EMBER_TRY_SCENARIO=ember-lts-2.8
-  - EMBER_TRY_SCENARIO=ember-lts-2.12
-  - EMBER_TRY_SCENARIO=ember-lts-2.16
-  - EMBER_TRY_SCENARIO=ember-release COVERAGE=true # Only log coverage from release
-  - EMBER_TRY_SCENARIO=ember-beta
-  - EMBER_TRY_SCENARIO=ember-canary
-  - EMBER_TRY_SCENARIO=with-ember-test-helpers
-  - EMBER_TRY_SCENARIO=with-@ember/test-helpers
+  global:
+    # See https://git.io/vdao3 for details.
+    - JOBS=1
+  matrix:
+    - EMBER_TRY_SCENARIO=node-tests
+    - EMBER_TRY_SCENARIO=ember-lts-2.4
+    - EMBER_TRY_SCENARIO=ember-lts-2.8
+    - EMBER_TRY_SCENARIO=ember-lts-2.12
+    - EMBER_TRY_SCENARIO=ember-lts-2.16
+    - EMBER_TRY_SCENARIO=ember-release COVERAGE=true # Only log coverage from release
+    - EMBER_TRY_SCENARIO=ember-beta
+    - EMBER_TRY_SCENARIO=ember-canary
+    - EMBER_TRY_SCENARIO=with-ember-test-helpers
+    - EMBER_TRY_SCENARIO=with-@ember/test-helpers
 
 matrix:
   fast_finish: true
@@ -33,15 +38,17 @@ branches:
     - "master"
 
 before_install:
-  - npm install -g bower
-  - bower --version
+  - npm config set spin false
+  - npm install -g npm@4
+  - npm --version
 
 install:
-  - bower install
   - yarn install --no-lockfile
 
 script:
-  - node_modules/.bin/ember try:one $EMBER_TRY_SCENARIO test --skip-cleanup
+  # Usually, it's ok to finish the test scenario without reverting
+  #  to the addon's original dependency state, skipping "cleanup".
+  - node_modules/.bin/ember try:one $EMBER_TRY_SCENARIO --skip-cleanup
 
 after_success:
   - if [ -f ./coverage/lcov.info ]; then cat ./coverage/lcov.info | ./node_modules/coveralls/bin/coveralls.js; fi

addon/-private/better-errors.js

@@ -1,6 +1,9 @@
 import Ember from 'ember';
+import EmberError from '@ember/error';
 import Ceibo from 'ceibo';
 
+const { Logger } = Ember;
+
 export const ELEMENT_NOT_FOUND = 'Element not found.';
 
 /**
@@ -29,6 +32,6 @@ export function throwBetterError(node, key, msg, { selector } = {}) {
     fullErrorMessage = `${fullErrorMessage}\n  Selector: '${selector}'`;
   }
 
-  Ember.Logger.error(fullErrorMessage);
-  throw new Ember.Error(fullErrorMessage);
+  Logger.error(fullErrorMessage);
+  throw new EmberError(fullErrorMessage);
 }

addon/-private/compatibility.js

@@ -1,12 +1,72 @@
-export const wait = (function() {
-  const hasLegacyEmberTestHelpers = window.require.has('ember-test-helpers');
-  const hasLatestEmberTestHelpers = window.require.has('@ember/test-helpers');
+//
+// This is a wrapper around `@ember/test-helpers` that we need for compatibility
+// reasons. Apps and addons aren't supposed to depend directly on
+// `@ember/test-helpers`, but just use the one that their version of
+// `ember-qunit` or `ember-mocha` provides. This compatibility module does three
+// jobs for us:
+//
+// 1. Helps us determine if we are running an RFC232/268 test or not
+// 2. Provides the test helpers needed to run RFC232/268 tests
+// 3. Provides a `wait` implementation for non-RFC232/268 (legacy) tests
+//
+// To accomplish (1) and (2) we need to determine if `@ember/test-helpers` is
+// present. If it isn't, we can't possibly be running RFC232/268 tests because
+// they rely on it. If it is, then we need its `getContext()` method to see if
+// any of the the RFC232/268 setup methods have been called. So, to keep this
+// complexity encapsulated in this file, if `@ember/test-helpers` is not
+// present, we export a stub `getContext()` function that returns null,
+// indicating that we are not running RFC232/268 tests, and then the rest of the
+// addon code won't try to access any of the other `@ember/test-helpers`
+// helpers.
+//
+// To accomplish (3), we need to determine if `ember-test-helpers` is present.
+// Because it's built with legacy support, anytime `@ember/test-helpers` is
+// present, `ember-test-helpers` will also be present. So we can check for
+// `ember-test-helpers/wait` and export it if present. If it's not present, we
+// don't want to throw an exception immediately because acceptance tests don't
+// need it, so we export a `wait` function that throws an exception if and when
+// it's called.
+//
+// Once we drop support for pre-RFC268 tests, including all calls to `wait`, we
+// can delete this file and import `@ember/test-helpers` directly.
+//
 
-  if (!hasLegacyEmberTestHelpers && !hasLatestEmberTestHelpers) {
-    return function() {
-      throw new Error('ember-test-helpers or @ember/test-helpers must be installed');
+// When a module imports `require`, it gets a dynamically generated module that
+// handles relative imports correctly, so there's no way to get at it to stub it
+// from another module/test. So instead we use the global require, which is only
+// available via window.require, so our tests can stub it out.
+const { require } = window;
+
+let helpers;
+
+if (require.has('@ember/test-helpers')) {
+  helpers = require('@ember/test-helpers');
+} else {
+  helpers = {
+    getContext() {
+      return null;
     }
-  }
+  };
+}
+
+if (require.has('ember-test-helpers/wait')) {
+  // This is implemented as a function that calls `ember-test-helpers/wait`
+  // rather than just assigning `helpers.wait = require(...).default` because
+  // since this code executes while modules are initially loading, under certain
+  // conditions `ember-test-helpers/wait` can still be in the pending state
+  // at this point, so its exports are still undefined.
+  helpers.wait = (...args) => require('ember-test-helpers/wait').default(...args);
+} else {
+  helpers.wait = () => {
+    throw new Error('ember-test-helpers or @ember/test-helpers must be installed');
+  };
+}
 
-  return window.require('ember-test-helpers/wait')['default'];
-})();
+export let getContext = helpers.getContext;
+export let visit = helpers.visit;
+export let click = helpers.click;
+export let fillIn = helpers.fillIn;
+export let triggerEvent = helpers.triggerEvent;
+export let focus = helpers.focus;
+export let blur = helpers.blur;
+export let wait = helpers.wait;

addon/-private/create.js

@@ -4,14 +4,58 @@ import { assign } from './helpers';
 import { visitable } from './properties/visitable';
 import dsl from './dsl';
 
+//
+// When running RFC268 tests, we have to play some tricks to support chaining.
+// RFC268 helpers don't wait for things to settle by defaut, but return a
+// promise that will resolve when everything settles. So this means
+//
+// page.clickOn('.foo');
+// page.clickOn('.bar');
+//
+// will not wait after either of the clicks, whereas
+//
+// await page.clickOn('.foo');
+// await page.clickOn('.bar');
+//
+// will wait after each of them. However, to preserve chaining behavior,
+//
+// page
+//   .clickOn('.foo')
+//   .clickOn('.bar');
+//
+// would need to wait between the clicks. However, if `clickOn()` just returned
+// `page` this would be impossible because then it would be exactly the same as
+// the first example, which must not wait between clicks.
+//
+// So the solution is to return something other than `page` from,
+// `page.clickOn('.foo')`, but something that behaves just like `page` except
+// waits for things to settle before invoking any async methods.
+//
+// To accomplish this, when building our Ceibo tree, we build a mirror copy of
+// it (the "chained tree"). Anytime a chainable method is invoked, instead of
+// returning the node whose method was invoked, we can return its mirror node in
+// the chained tree. Then, anytime an async method is invoked on that node
+// (meaning we are in a chaining scenario), the execution context can recognize
+// it as a chained node and wait before invoking the target method.
+//
+
 // See https://github.com/san650/ceibo#examples for more info on how Ceibo
 // builders work.
+
+// This builder builds the primary tree
 function buildObject(node, blueprintKey, blueprint, defaultBuilder) {
   blueprint = assign(assign({}, dsl), blueprint);
 
   return defaultBuilder(node, blueprintKey, blueprint, defaultBuilder);
 }
 
+// This builder builds the chained tree
+function buildChainObject(node, blueprintKey, blueprint, defaultBuilder) {
+  blueprint = assign({}, blueprint);
+
+  return buildObject(node, blueprintKey, blueprint, defaultBuilder);
+}
+
 /**
  * Creates a new PageObject.
  *
@@ -121,6 +165,22 @@ export function create(definitionOrUrl, definitionOrOptions, optionsOrNothing) {
   let { context } = definition;
   delete definition.context;
 
+  // Build the chained tree
+  let chainedBuilder = {
+    object: buildChainObject
+  };
+  let chainedTree = Ceibo.create(definition, assign({ builder: chainedBuilder }, options));
+
+  // Attach it to the root in the definition of the primary tree
+  definition._chainedTree = {
+    isDescriptor: true,
+
+    get() {
+      return chainedTree;
+    }
+  };
+
+  // Build the primary tree
   let builder = {
     object: buildObject
   };

addon/-private/dsl.js

@@ -10,12 +10,21 @@ import { isPresent } from './properties/is-present';
 import { isVisible } from './properties/is-visible';
 import { text } from './properties/text';
 import { value } from './properties/value';
+import { getRoot } from './helpers';
 import { wait } from './compatibility';
 
 const thenDescriptor = {
   isDescriptor: true,
   value() {
-    return (window.wait || wait)().then(...arguments);
+    // In RFC268 tests, we need to wait on the promise returned from the actual
+    // test helper, rather than a global method such as `wait`. So, we store the
+    // promise on the root of the (chained) tree so we can find it here and use
+    // it.
+    let promise = getRoot(this)._promise;
+    if (!promise) {
+      promise = (window.wait || wait)();
+    }
+    return promise.then(...arguments);
   }
 };
 

addon/-private/execution_context.js

@@ -1,18 +1,38 @@
 import { getContext } from './helpers';
+import isRfc268Test from './is-rfc268-test';
 import AcceptanceExecutionContext from './execution_context/acceptance';
 import IntegrationExecutionContext from './execution_context/integration';
+import Rfc268Context from './execution_context/rfc268';
 
 const executioncontexts = {
   acceptance: AcceptanceExecutionContext,
-  integration: IntegrationExecutionContext
+  integration: IntegrationExecutionContext,
+  rfc268: Rfc268Context
 };
 
 /*
  * @private
  */
 export function getExecutionContext(pageObjectNode) {
+  // Our `getContext(pageObjectNode)` will return a context only if the test
+  // called `page.setContext(this)`, which is only supposed to happen in
+  // integration tests (i.e. pre-RFC232/RFC268). However, the integration
+  // context does work with RFC232 (`setupRenderingContext()`) tests, and before
+  // the RFC268 execution context was implemented, some users may have migrated
+  // their tests to RFC232 tests, leaving the `page.setContext(this)` in place.
+  // So, in order to not break those tests, we need to check for that case
+  // first, and only if that hasn't happened, check to see if we're in an
+  // RFC232/RFC268 test, and if not, fall back on assuming a pre-RFC268
+  // acceptance test, which is the only remaining supported scenario.
   let testContext = getContext(pageObjectNode);
-  let context = testContext ? 'integration' : 'acceptance';
+  let context;
+  if (testContext) {
+    context = 'integration';
+  } else if (isRfc268Test()) {
+    context = 'rfc268';
+  } else {
+    context = 'acceptance';
+  }
 
   return new executioncontexts[context](pageObjectNode, testContext);
 }

addon/-private/execution_context/acceptance-native-events.js

@@ -12,14 +12,14 @@ export default function AcceptanceNativeEventsExecutionContext(pageObjectNode) {
 AcceptanceNativeEventsExecutionContext.prototype = Object.create(ExecutionContext.prototype);
 
 AcceptanceNativeEventsExecutionContext.prototype.visit = function() {
-  return visit(...arguments);
+  visit(...arguments);
+  return this.pageObjectNode;
 };
 
 AcceptanceNativeEventsExecutionContext.prototype.runAsync = function(cb) {
   (window.wait || wait)().then(() => {
     cb(this);
   });
 
-  return this.pageObjectNode;
+  return this.chainable();
 };
-

addon/-private/execution_context/acceptance.js

@@ -26,6 +26,10 @@ AcceptanceExecutionContext.prototype = {
       cb(this);
     });
 
+    return this.chainable();
+  },
+
+  chainable() {
     return this.pageObjectNode;
   },
 
@@ -56,7 +60,7 @@ AcceptanceExecutionContext.prototype = {
     triggerEvent(selector, container, 'change');
   },
 
-  triggerEvent(selector, container, eventName, eventOptions) {
+  triggerEvent(selector, container, options, eventName, eventOptions) {
     /* global triggerEvent */
     triggerEvent(selector, container, eventName, eventOptions);
   },

addon/-private/execution_context/integration-native-events.js

@@ -1,8 +1,6 @@
+import { run } from '@ember/runloop';
 import ExecutionContext from './native-events-context';
 
-import Ember from 'ember';
-const { run } = Ember;
-
 export default function IntegrationNativeEventsExecutionContext(pageObjectNode, testContext) {
   ExecutionContext.call(this, pageObjectNode, testContext);
 }
@@ -16,6 +14,5 @@ IntegrationNativeEventsExecutionContext.prototype.runAsync = function(cb) {
     cb(this);
   });
 
-  return this.pageObjectNode;
+  return this.chainable();
 };
-