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

Adapters #511

Merged
merged 9 commits into from
Jan 6, 2021
Merged

Adapters #511

merged 9 commits into from
Jan 6, 2021

Conversation

ro0gr
Copy link
Collaborator

@ro0gr ro0gr commented Jun 20, 2020

Implements #479

With this PR we convert private ExecutionContexts to public Adapters.

It's required now to setAdapter before start using page objects in tests:

// tests/test-helper.js

import DefaultAdapter from 'ember-cli-page-object/adapters/rfc268';
import { setAdapter } from 'ember-cli-page-object/adapters';

setAdapter(new DefaultAdapter());

in case you you need multiple adapters being set for different modules,
make sure to set a teardown it before each test:

  hooks.beforeEach(function() {
    setAdapter(new MyAdapter());
  });

  hooks.afterEach(function() {
    setAdapter(new DefaultAdapter());
  });

if no adapter is set, an error throws.

This PR does also remove few of the old and deprecated ModuleFor-only related APIs, like:

 - `getContext(`, `setContext(`,`removeContext(` - can be  replaced with `setAdapter(`
 - `render(`
 - `page.context`
 - `useNativeEvents(` - can be  replaced with `setAdapter(`
 - `andThen` behavior support. Now you have to explicitly wait for the previous action completeness.

And some APIs that just don't make sense anymore:

 - `registerExecutionContext`

@coveralls
Copy link

Coverage Status

Coverage decreased (-3.4%) to 95.031% when pulling d6fc9d8 on ro0gr:adapters into 242e2ab on san650:v2-beta.

@ro0gr
Copy link
Collaborator Author

ro0gr commented Nov 22, 2020

There are some work-arounds for old test scenarios. Though in general I'd like to get rid of legacy stuff like moduleFor* adapters, and old Ember version in scope of v2. However, I'd prefer to make sure it's possible to support via user land adapters, before removal from the code base.

In order to set a custom adapter, you can do it globally in global your test
helper, like:

```js
// tests/test-helper.js

import MyAdapter from 'path/to/adapter';
import { setAdapter } from 'ember-cli-page-object/adapters';

setAdapter(new MyAdapter());
```

in case you you need multiple adapters being set for different modules,
make sure to set a teardown it before each test:

```js
  hooks.beforeEach(function() {
    setAdapter(new MyAdapter());
  });

  hooks.beforeEach(function() {
    setAdapter(null);
  });
```

by default, if no adapter set, `RFC268Adapter` is used.

This PR does also introduce few of the old and deprecated
ModuleFor-only related APIs, like:

     - `getContext(`
     - `setContext(`
     - `removeContext(`
     - `render(`
     - `page.context`
     - `useNativeEvents(`
     - `andThen` behavior support. Now you have to explicitly wait for the
     previous action to complete.

And some APIs that just don't make sense anymore:

     - `registerExecutionContext`
old ember was supposed to be used with ember-cli-qunit according to the blueprint.

Seems like because of old ember + ember-qunit combo, we have some awaiting test issues.

This change allows us to get rid of a long living `expect-ember-error` work-around.
instead of relying on eternal waiters.

leveraging helpers like `wait`, `andThen`, and `settled`
is not reliable across different test frameworks/ember versions.

We should focus on testing our public API behavior at the first place.
there is a custom build flow to make the addon-test-support importable
from the root of the package. This seems to be a bit more complicated to
maintain then it could have been. Maybe exploring the Emberoider land
could help us to simplify here.

if (require.has('@ember/test-helpers')) {
// intentionally not using our local extension in order to make
// sure, RFC268 works by default, w/o Adapter being set.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

copy-pasted comment not applicable

ro0gr added 2 commits January 3, 2021 18:15
While it feels natural to auto-select an offcial RFC268 test mode,
if no adapter was specified; it adds some extra internal complexity,
because of eager import of the `@ember/test-helpers`.
In theory, it's possible to not have `@ember/test-helpers` installed,
which is demonstrated in "with-ember-test-helpers" ember-try scenario.

To solve this we could use `window.require(`, to lazily import needed helpers,
only in case if `@ember/test-helpers` installed. But this aproach may leads
to some issues in Node.js envs, since `window.require` is not a thibng there.
Also I'm not sure if `window.require(` is supposed to work fine with Emberoider.

So, we go an explicit way for now, and require to import,
and set a desired adapter on the test suite side manually.
@ro0gr ro0gr marked this pull request as ready for review January 3, 2021 22:47
@ro0gr ro0gr merged commit 8ef05a4 into san650:v2-beta Jan 6, 2021
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.

2 participants