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

feat: provide getPageTitle test helper to consuming apps #209

Merged
merged 4 commits into from
Jan 20, 2021

Conversation

srsgores
Copy link
Contributor

This PR allows consuming apps to assert a page's title (whether integration or acceptance test).

Usage documented in the README.md:

import { getPageTitle } from 'ember-page-title/test-support';

module('Acceptance | Register Page', function(hooks) {
  setupApplicationTest(hooks);

  test('visiting /register', async function(assert) {
    const registerURL = '/register';
    await visit(registerURL);

    assert.equal(currentURL(), registerURL);
    assert.equal(getPageTitle(), 'Register | Some Website');
  });
});

Copy link
Contributor

@knownasilya knownasilya left a comment

Choose a reason for hiding this comment

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

Some doc updates, otherwise looks good.

README.md Outdated Show resolved Hide resolved
Copy link
Contributor Author

@srsgores srsgores left a comment

Choose a reason for hiding this comment

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

Should be updated

@knownasilya
Copy link
Contributor

@raido thoughts on this addition? Since the helper was internal only before.

@srsgores
Copy link
Contributor Author

I'll clarify that part of our enterprise front-end is waiting on this PR to get merged. We had previously run our acceptance tests via a private helper as well, but this quickly became cumbersome when we needed to assert page titles on addon acceptance tests.

@knownasilya knownasilya merged commit 12817df into ember-cli:master Jan 20, 2021
@srsgores
Copy link
Contributor Author

@knownasilya, can we also bump package.json and release on npm?

@knownasilya
Copy link
Contributor

Done

@raido
Copy link
Contributor

raido commented Jan 20, 2021

I am a bit late here, but since this "private" helper is re-exported via index.js, it's fine. We can easily re-work internals.

We probably should add comment block so IDEs could give hints of expected parameters OR provide TypeScript interface file. I think proper JSDoc for starters would be nice.

This helper does have some internal logic comments in it, we should probably remove it or separate it still for testing specifically this addon vs public helper.

The Array.pop() workaround for consumers is not needed, it's only applicable for tests in this repository.

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.

3 participants