Ci

[stefanpenner]

• appveyor
• yarn
• no-bower
• remove stuff 1.0.0 wont have, and are causing issues on windows (Beta branch will include them still)
• make the gods happy

[kellyselden]

code LGTM. now lets just wait forever...

[stefanpenner]

[kellyselden]

Wanna take out .bowerrc too?

[stefanpenner]

travis node 7 tests appear to be unstable, largely due to npm install being run during the tests and timing out?

[stefanpenner]

Some of the windows tests appear to be failing due to the tests being racy, and NTFS not being very tolerant of such issues.

Those tests are super slow/brittle, even though it was very few tests fastboots tests are slower then all of ember-cli's tests. I suspect we should (soon) gives these tests some TLC. This would also improve iteration velocity on this add-on.

---

I think some of these tests also go away once the fastboot command is removed.

[stefanpenner]

So rather then creating new apps, why don't the tests just test this addons dummy app. That way we don't need to do nearly as much test setup, the addon's dummy app would become a living/tested example

[kratiahuja]

💯 The test setup is mostly the issue IMO since it creates a dummy ember app, copies over node modules into tmp, tries to cache it incorrectly and doing other stuff.

IMO the tests just need the assets for the serve type of tests (which are most of them). For build, it gets a bit tricky since it may need a setup...

[kratiahuja]

So rather then creating new apps, why don't the tests just test this addons dummy app. That way we don't need to do nearly as much test setup, the addon's dummy app would become a living/tested example

These are mostly node tests so yes we can leverage the dummy addon's build output to write such tests. Only occasionally some tests require npm install to be run in the dist folder (a fastboot feature which we want to remove anyway). I think once we remove FastBoot.require support which is also a security issue, we can leverage the dummy app in the addon build artifact in a straight forward way and write the node tests.

[stefanpenner]

Only occasionally some tests require npm install to be run in the dist folder (a fastboot feature which we want to remove anyway). I think once we remove FastBoot.require support which is also a security issue, we can leverage the dummy app in the addon build artifact in a straight forward way and write the node tests.

My vote is we kill this command asap, have it throw with a helpful assertion. Thoughts?

[kratiahuja]

My vote is we kill this command asap, have it throw with a helpful assertion. Thoughts?

💯 We already have an helpful message here that has been in here for atleast two beta releases. We can make this an error for the next beta with giving a message to use an alternative. Once we give this an error, we can easily switch our tests to make it robust. Then after 2 releases we just 🔪 the command for good.

[stefanpenner]

@kratiahuja then we should likely cut a beta branch (for the release you suggest \w fastboot command still), and remove this from master asap, so we can unblock the windows tests faster.

[stefanpenner]

@kratiahuja then we should likely cut a beta branch (for the release you suggest \w fastboot command still), and remove this from master asap, so we can unblock the windows tests faster.

I would just prefer to not fix tests for features we will shortly remove on windows ;)

[kratiahuja]

@stefanpenner +1 I don't think it is worth the effort to fix the windows test. I can work on throwing the error tomorrow evening. I also just realized that we don't have a beta branch cut. We have been releasing beta from master in which case master can no longer release betas (since it has my PR which is a rc candidate). I can cut a branch beta branch (before my commit) and send a PR against that branch. Sounds like a plan?

[stefanpenner]

I can cut a branch beta branch (before my commit) and send a PR against that branch. Sounds like a plan?

Ya that sounds wonderful!

[kratiahuja]

@stefanpenner This PR : #398 will fix the travis builds. I'll dig into the appveyor tomorrow/weekend.

[kratiahuja]

Ok, one god is now happy. Looking at making travis god happy now.

[stefanpenner]

that ipaddr error is so strange. Just in-case I have evicted the travis cache, and triggered a rebuild.

[stefanpenner]

@kratiahuja also, feel free to squash my commits down. We don't need all that noise (as most of the work has already landed via your other PR)

[stefanpenner]

cache clear + rebuild didn't seem to address.. very strange

[kratiahuja]

Yeah still looking into it since master is passing on npm.

[stefanpenner]

Most likely an issue with yarn being used, but npm being used for the addon test stuff.

Does it makes sense to move away from the addon test stuff now, instead testing the dummy app?

[stefanpenner]

Or is that premature?

[kratiahuja]

Yeah it's because of yarn that the issue is occuring. It's even introducing instability in appveyor too. See fix-travis branch. I think for now it would be better to use npm and then switch tests and enable yarn.

[kratiahuja]

Most likely the issue with yarn is this function: https://github.com/tomdale/ember-cli-addon-tests/blob/master/lib/utilities/pristine.js#L130

[kratiahuja]

@stefanpenner both gods should be happy now. we can switch to using yarn once we move tests to dummy addon. appveyor is also unstable due to yarn (see this run).

We can merge this (after I squash the commits now) and I'll work on moving the tests in a different PR (since it will large PR). Does that sound good with you?

[stefanpenner]

Sounds great. Squash out my commits. Then feel free to merge!

[kratiahuja]

Squashed. Merging now.