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

[Enhancement] Ember-try & parallel travis-ci scenario tests for addons #3921

Merged
merged 1 commit into from
Apr 20, 2015
Merged

[Enhancement] Ember-try & parallel travis-ci scenario tests for addons #3921

merged 1 commit into from
Apr 20, 2015

Conversation

mike-north
Copy link
Member

Thanks to @kategengler's add-on, inspired by @ef4's work for liquid-fire, we can easily test apps and addons against multiple versions of ember.js

This PR enhances the existing ember-try support for addons with a default configuration for testing against:

  • 1.10 "default" -- may require some modification to ember-try
  • components/ember#release
  • components/ember#beta
  • components/ember#canary

Additionally, the default travis-ci configuration will test these scenarios in parallel, while allowing builds to pass, even if tests fail using ember-canary. Benefits I see in this are:

  • Developers get feedback on specific builds easily
  • Test jobs are potentially run in parallel, depending on travis-ci worker node availability
  • Apps and addons are tested against canary by default, with no downside to the developer. Build times are no longer than they otherwise would be, test failures in canary are tolerated
  • Using the component/ember channels for scenarios instead of numbered versions allows the release team to get feedback on new releases (including canary builds) without any action required by addon and app developers

screen shot 2015-04-17 at 10 52 19 am

TODO

  • Updates to .travis.yml
    • Rename environment variable to EMBER_TRY_SCENARIO
    • script: ember try $EMBER_TRY_SCENARIO test
    • Append build matrix
      • Append env matrix
    • Ensure that config/ember-try.js is in place for addons

module.exports = {
scenarios: [
{
name: 'ember-1.10',
Copy link
Member Author

Choose a reason for hiding this comment

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

Including this scenario feels like ember-cli would have the opinion of encouraging 1.10 compatibility. I'd like some specific feedback on whether people feel this is appropriate

@mike-north mike-north changed the title [WIP - Enhancement] Ember-try & parallel travis-ci scenario tests [Enhancement] Ember-try & parallel travis-ci scenario tests Apr 17, 2015
@rwjblue
Copy link
Member

rwjblue commented Apr 17, 2015

This seems somewhat odd to me, why would we advocate for testing an app against multiple versions of Ember?

This change makes makes each version tested in separate Travis builds (which is nice for sure), but we are already testing with ember try:testall by default for newly generated addons (via the npm test script).

@mike-north
Copy link
Member Author

Ah, ok. I'll go ahead and update this so that it only affects addons. There's still a big advantage to running individual ember try $SCENARIO test in parallel as opposed to a single serial critical path.

@@ -9,6 +9,17 @@ cache:
directories:
- node_modules

env:
- E_SCENARIO=ember-1.10
Copy link
Member

Choose a reason for hiding this comment

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

I think we should stick to just checking the channels and the "default" (which may require some tweaks in ember-try).

Also, can we make the environment variable EMBER_TRY_SCENARIO (to me a bit more explicit)?

Copy link
Member

Choose a reason for hiding this comment

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

I submitted ember-cli/ember-try#21 to make the default config more in line with this.

Could you add the "default" scenario in the config/ember-try.js (like from my PR above) instead of ember-1.10?

@rwjblue
Copy link
Member

rwjblue commented Apr 17, 2015

This seems somewhat odd to me, why would we advocate for testing an app against multiple versions of Ember?

Welp, apparently it helps if I read the nice description you wrote. Sorry about that.


I left an inline comment/tweak, but other than that I am 👍 (and sorry for not paying close enough attention).

@mike-north mike-north changed the title [Enhancement] Ember-try & parallel travis-ci scenario tests [Enhancement] Ember-try & parallel travis-ci scenario tests for addons Apr 17, 2015
@alexdiliberto
Copy link
Contributor

Overall 👍 for Addons, However I think opting-in to testing against Canary may be a little much. I would go with Beta and Release...I could be in the minority though

@mike-north
Copy link
Member Author

@alexdiliberto would you want to test your addon against canary if build times were no longer, and it didn't get in the way of your development workflow?

@rwjblue
Copy link
Member

rwjblue commented Apr 17, 2015

I think we should definitely encourage (with allowed failures) tests against canary. It would serve to give you advanced warning of upcoming issues that you will face, but I'm not 100% opposed to testing apps only against release and beta channels (but keeping addons with all three).

@alexdiliberto
Copy link
Contributor

I'm definitely on board with the premise here, @rwjblue You mentioned allowed failures, I didn't consider that, I think that would work well here. I didn't want everyone to go crazy when their tests fail for some little thing on canary and travis kept complaining

@stefanpenner
Copy link
Contributor

Im +1 for add-ons.

I would love to consider this for apps as well, it would be valuable to see if you app works on the current version, the latest beta and maybe the latest canary? Of-course this seems like an opt-in thing? But it may help catch issues early, also help developers future proof there apps.

@mike-north
Copy link
Member Author

ok this is working now, with the exception of:

  • ember-try understanding the default version

@rwjblue
Copy link
Member

rwjblue commented Apr 17, 2015

See my example in the referenced PR you m can implement the ember-try config file now.

@rwjblue
Copy link
Member

rwjblue commented Apr 17, 2015

TL DR add default with no dependencies in the config file.

@mike-north
Copy link
Member Author

Thanks @rwjblue. Just ran it locally and all scenarios are run and complete successfully. Travis works properly with the generated .yml file, so I think we're good to go

@mike-north
Copy link
Member Author

PR build was failing due to stale npm cache, and is passing now

@rwjblue
Copy link
Member

rwjblue commented Apr 20, 2015

👍

stefanpenner added a commit that referenced this pull request Apr 20, 2015
[Enhancement] Ember-try & parallel travis-ci scenario tests for addons
@stefanpenner stefanpenner merged commit 764603a into ember-cli:master Apr 20, 2015
@stefanpenner
Copy link
Contributor

rad :)

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.

None yet

4 participants