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] enable external partner testing #5567

Merged
merged 1 commit into from
Aug 16, 2018
Merged

[FEAT] enable external partner testing #5567

merged 1 commit into from
Aug 16, 2018

Conversation

runspired
Copy link
Contributor

Creates a script that enables us to run the tests of external apps and addons against commits in ember-data.

const projectRoot = path.resolve(__dirname, '../../');
const externalProjectName = process.argv[2];
const gitUrl = process.argv[3];
const tempDir = path.join(projectRoot, '../__external-test-cache');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I intend to set this up such that we can cache the repositories we test on travis and use git pull to get latest most of the time vs needing to blow away and start over all the time.

Copy link
Member

Choose a reason for hiding this comment

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

Alternatively, you could shallow clone for some speed improvement and not have to worry about caching mechanisms.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kategengler how does that work?

Copy link
Member

Choose a reason for hiding this comment

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

git clone --depth=1 <repo> will get you just the HEAD of the repo, so you don't get all history.


// run project tests
console.log(`Running tests for ${externalProjectName}`);
execWithLog(`cd ../__external-test-cache/${externalProjectName} && ember test`, true);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm unsure if an unsuccessful test run would cause us to throw an error and fail, likely need to do more here.

@runspired
Copy link
Contributor Author

runspired commented Aug 14, 2018

cc @backspace @kategengler @jakesjews :)

@kategengler
Copy link
Member

Ah, this may be painful/brittle if the external apps fail for reasons unrelated to ember-data. My approach was going to be to add ember-try config to the various apps to test against canary ember-data + create a dashboard with the status of the various CI results. That doesn't get PR testing, though.

@runspired
Copy link
Contributor Author

@kategengler that would still be immensely useful; however, due to certain constraints around 3.4 LTS and internal branching for RecordData, @hjdivad and I decided it would be immensely useful to have commit-over-commit testing that would allow us to prevent release if needed.

Obviously we would need to check if the tests failed due to our changes or those of the external app. One thought I have for reducing false failures is to run the external apps tests twice, once against it's own desired version and once against the new commit, and only fail if the first passes but the second doesn't.

@backspace
Copy link
Contributor

run the external apps tests twice

This seems like a good plan. Maybe I’ll finally figure our the intermittently-failing race-condition-y test 😳

Nice use of stages for this!

I recently updated travis-web to use Node 10, hopefully that‘s not eventually a problem with this using 6. The only thing I tend to have problems with across Node versions is ember-cli-sass.

.travis.yml Outdated
script: yarn test-external:emberaddons.com
- env: NAME=ember-data-change-tracker
script: yarn test-external:ember-data-change-tracker
- env: NAME=ember-m3
Copy link
Contributor

Choose a reason for hiding this comment

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

re all these NAME= workarounds, you can now explicitly name jobs using name: "Whatever", as with the “Max Transpilation” one earlier in the file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🗡 🔥 🔋

/* eslint-disable no-console, node/no-extraneous-require, node/no-unpublished-require */
const fs = require('fs');
const path = require('path');
const { execSync } = require('child_process');
Copy link
Member

Choose a reason for hiding this comment

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

lets use execa as it cleans up leaked children, if the script is exited early.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

.travis.yml Outdated

# runs tests against various open-source projects for early-warning regression analysis
- stage: external partner tests
name: "Model Fragments"

Choose a reason for hiding this comment

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

It would be great to see Ember Data Factory Guy added to this list

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

@runspired runspired dismissed stefanpenner’s stale review August 15, 2018 18:47

feedback has been addressed and a new review is needed

add names to external partner tests

use shallow cloning, use bower if necessary, add more external partners

nice names

smoke tests

prettier and storefront

use execa

fix bower command

use node 10 in travis
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.

5 participants