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

Update blueprint to include tests/index.html. #153

Merged
merged 4 commits into from Nov 5, 2018

Conversation

rondale-sc
Copy link
Contributor

@rondale-sc rondale-sc commented Oct 22, 2018

[Edit from latest commit]

The following is meant to satisfy the PR in glimmer application pipeline
found here:

glimmerjs/glimmer-application-pipeline#152

That PR takes the output from the <glimmerAppRoot>/tests/index.html
and bundles it with the appropriate testEntryPoint in the dist after a
build.

(Sidenote: it does this in all non production environments
rather than just test)

In order for the process for new Glimmer apps to be smooth we need to
utilize that PR (from glimmer application pipeline) and then also do the
following (implemented in this PR):

  • Bump package version of glimmer-application-pipeilne (the assumption
    of this PR is that when we release glimmer-application-pipeline it will
    be bumped as a patch version to 0.11.3
  • Add the default tests/index.html to the root of the project. This
    is what will be used by glimmer-application-pipeline
    • note here it uses a CDN url for QUnit because that is easier in a
      future PR we'll have glimmer-application-pipeline handle this such
      that it is available in the dist
  • Finally we must update the testem.json to point to tests/index.html
    which (as a result of how ember-cli works) is relative to the dist
    folder already.

The benefits of this is that we can get a modern version of QUnit.
Until this point new glimmer applications could only have 1 function
that consumes the hooks per module which is stifling for a number of
reasons. Furthermore, this PR (and its associated
glimmer-application-pipeline PR) bring glimmer applications closer to
the harness of Ember itself.

The following is meant to satisfy the PR in glimmer application pipeline
found here:

glimmerjs/glimmer-application-pipeline#152

That PR takes the output from the `<glimmerAppRoot>/tests/index.html`
and bundles it with the appropriate testEntryPoint in the dist after a
build.

(Sidenote: it does this in all non production environments
rather than just test)

In order for the process for new Glimmer apps to be smooth we need to
utilize that PR (from glimmer application pipeline) and then also do the
following (implemented in this PR):

- Bump package version of glimmer-application-pipeilne (the assumption
of this PR is that when we release glimmer-application-pipeline it will
be bumped as a patch version to 0.11.3
- Add the default `tests/index.html` to the root of the project.  This
is what will be used by glimmer-application-pipeline
   - *note here* it uses a CDN url for QUnit because that is easier in a
   future PR we'll have glimmer-application-pipeline handle this such
   that it is available in the dist
- Finally we must update the testem.json to point to `tests/index.html`
which (as a result of how ember-cli works) is relative to the `dist`
folder already.

---

The benefits of this is that we can get a modern version of QUnit.
Until this point new glimmer applications could only have 1 function
that consumes the `hooks` per module which is stifling for a number of
reasons.  Furthermore, this PR (and its associated
glimmer-application-pipeline PR) bring glimmer applications closer to
the harness of Ember itself.
<head>
<meta charset="utf-8">
<meta http-equiv="X-UA-Compatible" content="IE=edge">
<title>Glimmer PDP Viewer Tests</title>
Copy link
Member

Choose a reason for hiding this comment

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

Seems incorrect, probably should be <%= name %>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Haha, quite right.

<meta name="viewport" content="width=device-width, initial-scale=1">

<link rel="stylesheet" href="https://code.jquery.com/qunit/qunit-2.6.2.css">
<script src="https://code.jquery.com/qunit/qunit-2.6.2.js"></script>
Copy link
Member

Choose a reason for hiding this comment

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

Lets go to 2.8.0 (for both this script and the styles above) 😸

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea

@rwjblue rwjblue merged commit 030ea49 into glimmerjs:master Nov 5, 2018
@rwjblue rwjblue changed the title Bump glimmer-application-pipeline dependency Update blueprint to include tests/index.html. Nov 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants