san650 / ember-cli-page-object

This ember-cli addon eases the construction of page objects on your acceptance and integration tests
http://ember-cli-page-object.js.org/
MIT License
275 stars 90 forks source link

Improve assertions #392

Closed ro0gr closed 6 years ago

ro0gr commented 6 years ago

I think there is a lack of assertions expressiveness using page objects:

// .ok() results with a very poor assertion message which is context-less.
// expected: true
// actual: false
assert.ok(page.title.isVisible, 'DocsHero');

// .equal() also leads to a non-contextual message
// expected: 'DocsHero'
// actual: 'NotAHero'
assert.equal(page.title.text, 'DocsHero');

I think we can get an inspiration from qunit-dom. It has very similar API:

What if we have own qunit extension like:

// expected: page title is visible
// actual: page title is not visible
assert.PO(page.title).isVisible(); // use of PO just for visibility

// expected: page title has text "DocsHero"
// actual: page title has text "NotAHero"
assert.PO(page.title).hasText('DocsHero');

assert.PO(page.title).hasAttribute('aria-role', 'button');

This way an assertion can get all the necessary information to provide the best message possible

Alternatives

Better integration with qunit-dom

There is also an option to connect page-object with qunit-dom. But page object doesn't provide a way to get a DOM element. If we even expose an element then assertion would become too verbose:

  assert.dom(page.title.element).hasText('DocsHero');

Also there will not be an ability to support custom(defined by user page.isOnTop) page object props.

ember-qunit-nice-errors

We can recommend to use "ember-qunit-nice-errors" for better error messages. AFAIK it instruments qunit assertion lines in the build time to provide an assertion source code as an assertion message. So for the following test:

test('it works', function(assert) {
  assert.equal(page.title.text, 'DocHero');
});

you would get the following assetion message:

  assert.equal(page.title.text, 'DocHero');
  expected: 'DocHero'
  actual: 'NotAHero'

I feel like we still can improve readability of the tests code and assertion messages in runtime.

san650 commented 6 years ago

@ro0gr have you tried ember-qunit-nice-errors ? This other addon was my attempt to solve the same problem. It's qunit specific though.

ro0gr commented 6 years ago

@san650 thanks! Updated "Alternatives" section to include "ember-qunit-nice-errors"

sukima commented 6 years ago

Alternative: Sinon offers a good assertion API which integrates into your test framework. Using this style API made it easy to offer good error messages but also keep the library frame agnostic (has hooks for QUnit, Mocha, Jasmine, etc.) I found this exceptionally useful in my own tests that use Sinon.

What this would look like

All page objects would have an assert namespace. Each assert would call the frameworks' global assert hook. For example a simple isVisible could look like this:

test('...', function() {
  page.assert.ok(page.title.isVisible);
})

Implementation

PageObject has an assert namespace with pass and fail methods. By default they are defined in the addon:

PageObject.assert.fail = function(msg) {
  throw new AssertionError(msg);
};
PageObject.assert.pass = function(msg) {
  // No-op
};

Then in a framework specific addon (i.e. ember-cli-page-object-qunit):

import QUnit from 'qunit';

PageObject.assert.fail = function(msg) {
  QUnit.assert.ok(false, msg);
};
PageObject.assert.pass = function(msg) {
  QUnit.assert.ok(true, msg);
};

The actual PageObject assertion API could look something like:

PageObject.assert.ok = function(descriptor) {
  let keys = [];
  let node = descriptor;
  do {
    keys.unshift(Ceibo.meta(node).key);
  } while (node = Ceibo.parent(node));
  let msg = `expected ${keys.join('.')} to be true`;
  if (descriptor) {
    PageObject.assert.pass(msg);
  } else {
    PageObject.assert.fail(msg);
  }
};

Maybe? Anyway take a look at how Sinon tackled this very same problem and I think their API solution would fit well in this use case.

ro0gr commented 6 years ago

@sukima I'm glad you also think assertions can be improved.

Regarding this snippet:

test('...', function() {
  page.assert.ok(page.title.isVisible);
})

page.title.isVisible returns a boolean so if we do like this we would be unable to get any context specific information:

I think to keep a sinon flavour it has to be re-written to something like:

test('...', function() {
  page.assert.isVisible(page.title);
})

which is pretty similar to the original proposal.

wynnfarm commented 6 years ago

I was just wondering if anyone else has heard any discussions pertaining to this topic, as quint-dom will become part of the ember blue print in 3.2.

ro0gr commented 6 years ago

As mentioned in the Alternatives it is potentially possible to integrate with qunit-dom.

The work on qunit asserionts improvements is going(slooowly) in the ember-qunit-page-object repo.

The idea is to provide API like:

  assert.po(component)
    .isVisible()
    .hasText('some text')
    .not.contains('O_o'); // not implemented yet

Closing the issue cause it's totally possible to do it in a separate addon.