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

API Suggestion: Multi-Attribute fillables #105

Closed
richmolj opened this issue Jan 25, 2016 · 13 comments
Closed

API Suggestion: Multi-Attribute fillables #105

richmolj opened this issue Jan 25, 2016 · 13 comments

Comments

@richmolj
Copy link

Just ran into this and it's a small issue but since you are working towards v1.0 figured I'd drop a line.

I have a search form where I'd like to enter values into the inputs, but also assert on those values (like loading the form from server data and verifying defaults are filled in). Currently this requires two attributes:

export default PageObject.create({
  nameValue: value('#search_name'), // for loading from server and asserting the value
  name: fillable('#search_name'), // for querying
});

It would be nice to have just one 'name' field I could re-use. Maybe something like

page.name('Homer'); // fills in
page.name(); // returns 'Homer';

I could see this applying to things like clickables as well, where you want to assert on the button label as well as click the button.

@juanazam
Copy link
Collaborator

@richmolj right now the closes thing you can do is

export default PageObject.create({
  name: {
    scope: '#search_name',
    value: value(),
    fillIn()
  }
});

And then do:

  page.name().value();
  page.name().fillIn('Juan')

Anyway, I get the point and I understand the proposed API is simpler, I think we can discuss about it for 1.x.

Thanks!

@san650
Copy link
Owner

san650 commented Jan 27, 2016

@richmolj I agree this is something to consider. I'm not sure if I would change the behavior of fillable and value because fillable returns a promise and runs outside of the andThen block but value must be called inside a andThen block.

I think there's a way to create a simple helper that achieves that behavior, let me think about it a bit and I get back to you with an example on how to implement it using a customHelper.

Saying that, I think it would be nice to have a section in the site with examples of custom helpers that the community can use and contribute to, and if any of them becomes popular we can add it to the addon.

@richmolj @juanazam what do you think?

@san650
Copy link
Owner

san650 commented Jan 28, 2016

@richmolj here's an example on how to achieve that. I didn't test it thoroughly but I think it works just fine

var input = PageObject.customHelper(function(scope) {
  return function(textToFillIn) {
    if (typeof(textToFillIn) === 'undefined') {
      return findWithAssert(scope).val();
    } else {
      return fillIn(scope, textToFillIn);
    }
  };
});

var page = PageObject.create({
  visit: visitable('/users'),
  userName: input('#userName')
});

// ... test ...

page.visit().userName('san650');

andThen(function() {
  assert.equal(page.userName(), 'san650');
});

Please let me know if it works for you 😄

@richmolj
Copy link
Author

@san650 interesting, it does look like that would work. That said, I actually really liked the API of @juanazam's comment. I wonder if it would be possible to get this 'out of the box', by always returning a reference to the element, and always being able to call any action/query on it:

export default PageObject.create({
  name: PageObject.all('#name')
});

page.name().value();
page.name().fillIn('Juan');
page.name().click();
page.name().text();

So you save whether it's a clickable, fillable, etc until the actual test. I realize this would be a significant divergence though.

@juanazam
Copy link
Collaborator

@richmolj you can actually build your own component to do that.

tests/pages/components/input.js

export default function input(selector) {
  return {
    scope: selector,
    value: PageObject.value()
    fillIn: PageObject.filable()
  };
}
/tests/my-test.js
import input from './pages/components/input';

let page = PageObject.create({
  name: input("#name")
})

We have that in our application and it's very useful. Do you think this might do the trick?

@richmolj
Copy link
Author

Yep, I'm actually heading down that direction now. It would be nice to get this built-in to the add-on, though, so the API is consistent. For instance avoiding some projects doing page.name().val() and others doing page.name().getValue(), etc.

@juanazam
Copy link
Collaborator

@richmolj we have thought abut building some 'default' components withing the addon, but it seems to hard to find a set that will always be useful for everybody. Anyway, there is still room for discussion on that, what do you think @san650?

@san650
Copy link
Owner

san650 commented Jan 28, 2016

@richmolj @juanazam actually, we already generate several properties for each component. Please take a look at this page.

So, let's say you have the following definition

var page = PageObject.create({
  title: { scope: 'h1' }
});

then you have the following properties for free

test('...', function(assert) {
  // actions
  page.title().click();
  page.title().clickOn('a subtitle');

  andThen(function() {
    // predicates and queries
    assert.ok(page.title().isVisible());
    assert.ok(!page.title().isHidden());
    assert.ok(page.title().contains('foo bar'));
    assert.equal(page.title().text(), 'foo bar baz');
  });
});

Maybe it makes sense to keep adding default properties to DRY tests even more.

What other properties do you think we need? .fillIn, .value, .attribute...

@richmolj
Copy link
Author

@san650 I did not realize that! I think this is actually my preferred way to use this add-on for most things. Is there a downside to just adding all the others?

@san650
Copy link
Owner

san650 commented Jan 28, 2016

I don't see a downside, we introduced this feature with @juanazam a few months ago to see how it was going to work and personally I'm happy with the results, page object definitions got reduced a lot!

We chose the properties that made sense for most elements in the page, you usually want click, read the text or see if the element is there, at the time I felt that fillIn, value, etc. were too specific for some parts of the application... but now I feel different.

Feedback like yours is really helpful to help us define where we need to go with the addon, let's define what other attributes make sense to have as defaults.

Right now we are working on the 1.0 release and I don't want to keep adding features to it so we can release it sooner. But we can create a PR to merge right after the 1.0 release.

@juanazam
Copy link
Collaborator

@san650 sounds great! let's define that list so we can implement it as an 1.x feature.

@juanazam
Copy link
Collaborator

Hey @richmolj since lots of properties are now included by default on components. Do you think we can close this?

@richmolj
Copy link
Author

Yup 👍

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

No branches or pull requests

3 participants