Skip to content

Setup addon tests#171

Merged
xg-wang merged 1 commit into
ember-cli:masterfrom
xg-wang:fastboot
Dec 4, 2018
Merged

Setup addon tests#171
xg-wang merged 1 commit into
ember-cli:masterfrom
xg-wang:fastboot

Conversation

@xg-wang
Copy link
Copy Markdown
Member

@xg-wang xg-wang commented Nov 21, 2018

Introduce ember-cli-addon-tests to test:

  1. ember app with ember-cli-fastboot has dist/ember-fetch/fastboot-fetch.js built
  2. serve ember app with ember-cli-fastboot renders fetched content

@xg-wang
Copy link
Copy Markdown
Member Author

xg-wang commented Nov 22, 2018

Blocked by tomdale/ember-cli-addon-tests#198 and tomdale/ember-cli-addon-tests#199

Workarounds:

  1. install a fixed version ember-data rather than master
  2. npm run ember-cli-addon-tests

@xg-wang
Copy link
Copy Markdown
Member Author

xg-wang commented Nov 28, 2018

@Turbo87 @stefanpenner @rwjblue This is green now, can you review?

Comment thread test/fastboot-test.js
pkg.devDependencies['ember-cli-fastboot'] = '*';
// These 2 are in ember-fetch's package.json, symlinking to dummy won't help resolve
pkg.devDependencies['abortcontroller-polyfill'] = '*';
pkg.devDependencies['node-fetch'] = '*';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not sure I understand. Why are these needed?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

See this PR, Fastboot.requrie is reading from node_modules/node-fetch but it's under node_modules/ember-fetch/node-fetch when symlink.

Comment thread test/fastboot-test.js Outdated

let app;

before(function() {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this should use beforeEach() so that the tests can't leak state

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks, changed

Comment thread test/fastboot-build-test.js Outdated

let app;

before(function() {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this should use beforeEach() so that the tests can't leak state


loadInitializers(App, config.modulePrefix);

export default App;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

where are these fixture files being used?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

These are used by ember-cli-addon-tests to copy over generated new app. It's same setup as https://github.com/ember-fastboot/ember-cli-fastboot/tree/master/test

@xg-wang xg-wang force-pushed the fastboot branch 3 times, most recently from fcf0c9a to cef993a Compare December 3, 2018 05:55
use npm run instead of yarn in travis to work around
tomdale/ember-cli-addon-tests#198
Comment thread .travis.yml
- yarn lint:js
- yarn test
- yarn test:node
- npm run test:node
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why change this to npm?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is a known issue tomdale/ember-cli-addon-tests#198, should be fixed by a WIP PR tomdale/ember-cli-addon-tests#105

@xg-wang xg-wang merged commit 1c3a8d5 into ember-cli:master Dec 4, 2018
@xg-wang xg-wang deleted the fastboot branch December 4, 2018 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants