Skip to content

[FEAT] enable external partner testing - #5567

Merged
runspired merged 1 commit into
masterfrom
add-test-infra
Aug 16, 2018
Merged

[FEAT] enable external partner testing#5567
runspired merged 1 commit into
masterfrom
add-test-infra

Conversation

@runspired

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
Contributor

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.

Comment thread lib/scripts/test-external.js Outdated

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

Copy link
Copy Markdown
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

runspired commented Aug 14, 2018

Copy link
Copy Markdown
Contributor Author

cc @backspace @kategengler @jakesjews :)

@kategengler

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
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.

Comment thread .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
Copy Markdown
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
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🗡 🔥 🔋

Comment thread lib/scripts/test-external.js Outdated
/* 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
Copy Markdown
Contributor

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
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Comment thread .travis.yml Outdated

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

Copy link
Copy Markdown

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
Copy Markdown
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
@runspired
runspired merged commit db9cf7a into master Aug 16, 2018
@runspired
runspired deleted the add-test-infra branch August 16, 2018 05:39
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