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

Chaining from the root #433

Open
ro0gr opened this issue Nov 22, 2018 · 4 comments
Open

Chaining from the root #433

ro0gr opened this issue Nov 22, 2018 · 4 comments
Labels

Comments

@ro0gr
Copy link
Collaborator

ro0gr commented Nov 22, 2018

Problem

In the guides we currently have definitions like:

{
    scope: '.awesome-form',
    firstName: fillable('#firstName'),
    lastName: fillable('#lastName'),
    submit: clickable('button')
}

which are used in the following way:

await form
  .firstName('John')
  .lastName('Doe')
  .submit();

The problem with the definition is that once we need some additional functionality, like getting a value of input or additional actions, we have to duplicate selectors and expand definition with new properties names which may be hard to follow when you write a test:

{
  firstName: fillable('#firstName'), // weird name from this point
  firstNameValue: value('#firstName'), // duplicated selector and weird name
  focusOnFirstName: focus('#firstName'), // duplicated selector and weird name
}

A better approach would be to use nested components for that:

{
    firstName: { scope: '#firstName' },
    lastName: { scope: '#lastName' },
    submit: clickable('button')
}

However we are not able to use chaining anymore, cause after an action is invoked we are only permitted to continue chaining on the object where the action was called/defined. So the test has to be re-written in the following way:

await form.firstName.fillIn('John')
await form.lastName.fillIn('Doe')
await form.submit();

Proposal

I'd like to be able to have a chaining support when I use nested components, so I can write a test like:

await form
  .firstName.fillIn('John')
  .lastName.fillIn('Doe')
  .submit();

In order to make it possible actions should change behavior from returning a current node to returning a page object root as a context for the following chain.

Note: A page object root is a node on which create was called:

  const a = create({
    b: {
      c: {}
    }
  })

  // `a` is a root of both `a.b` and `a.b.c`

I'm not sure yet how much does it take to implement, but I'm pretty confident it should noticably simplify some of internals.

Drawbacks

The main drawback is that it doesn't seem possible to make it in a backward compatible way. We can try to add a codemod transform to ease the transition. However, in my opinion, the change should not be included in v1.

The other drawback is that in some cases it may be unclear that we have a root at all:

const { username } = form;
// ... codes...

await username
  .fillIn('something')
  .blur(); // it is invoked on the "form" component!

In this is case it feels natural to expect that blur would perform on a username page object.
However it would be called on the root form page object.

Additionally sometimes I assume someone really wants to keep chaining on a specific level of nesting(change the root). For such cases I can think of an additional API to fork a page object with the new root but preserving parent scopes:

const { username } = form;
// ... codes...

await username.fork()
  .fillIn('something')
  .blur();

Alternatives

Expose root property

A way to provide more control over a chainable node can be to expose a root property for each page object:

await form.firstName.fillIn('John')
  .root.lastName.fillIn('Doe')
  .root.submit();

I find it a bit clumsy, but it can be added in a backward compatible way.

Do nothing

If we do nothing, then we don't have a way to use chaining with a full power. Currently we don't have a way to access parent attributes once we've interacted with a nested component which is quite limiting.

@san650
Copy link
Owner

san650 commented Dec 4, 2018

@ro0gr for this one

const form = create({
    firstName: { scope: '#firstName' },
    lastName: { scope: '#lastName' },
    submit: clickable('button')
});

await form.firstName.fillIn('John')
await form.lastName.fillIn('Doe')
await form.submit();

Have you evaluated the possibility of using the as helper? (we already have it)

await form
  .firstName.as(input => input.fillIn('John'))
  .lastName.as(input => input.fillIn('Doe'))
  .submit();

I'm not sure if as helper will wait for the inner action to complete, if it doesn't, maybe we could update it to wait for the returning value to complete (if it's a promise) before moving to the next statement.

http://ember-cli-page-object.js.org/docs/v1.15.x/api/as

What do you think?

@san650 san650 added the feature label Dec 4, 2018
@ro0gr
Copy link
Collaborator Author

ro0gr commented Dec 5, 2018

@san650 it's interesting. I haven't considered it until have read your comment.

I'm pretty sure the as helper doesn't support inner asynchrony currently. But let's I assume it does.

I don't see a reason why it wouldn't work. However, I find the as helper usage non-optimal here. Let me try explain it...

As far as I can see, the purpose of the as helper is to save the user few keystrokes while performing multiple operation on some nested component:

myPage.header.searchBar.as((s) => {
  assert.ok(s.isVisible);
  assert.equal(s.text, 'something');
})

However, once you need to perform another set of operations on the same nested component, you have to repeatl the whole path to the component again:

myPage.header.searchBar.as((s) => {
  // another set of operations on the filter...
})

From the other side, I find it a useful pattern to simply have short-hands for the nested components which I frequently use in a specific test module:

// Make a shorthand once, at the very top of the test module 
const { searchBar } = myPage.header;

The searchBar still belongs to the root page object, it inherits its scope etc,. So I can use anywhere in my test module, like that:

assert.ok(searchBar.isVisible);
assert.equal(searchBar.text, 'something');

In other words, I believe, a simple JS assignment completelly solves the as purpose(please hit me if I lie) in general.

So I think in the current form of as, even with async support, is not the best choice for the task.

@ro0gr
Copy link
Collaborator Author

ro0gr commented Dec 5, 2018

I think that the proposed form of the fork is also not the best. It only allows you to assign, but not to destruct:

const username = form.username.fork();
const lastname = form.lastname.fork();

I think rather than forking the current node, we should fork all the immediate children. That would allow us to perform shorthands destruction in one line:

  const { username, lastname } = form.forkChildren(); // oh, this long, terrible name

and we can still do a regular assignment, if it's sufficient:

  const username = form.forkChildren().username;

@ro0gr
Copy link
Collaborator Author

ro0gr commented Dec 14, 2018

Updated description with the root property alternative.

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

No branches or pull requests

2 participants