From 9133c586116cee10687c73289b9ed59c21315b36 Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Mon, 25 Nov 2019 22:22:47 -0800 Subject: [PATCH 1/5] improve packaging --- bin/-tarball-info.js | 8 +-- bin/test-external-partner-project.js | 76 +++++++++++++++++----------- 2 files changed, 51 insertions(+), 33 deletions(-) diff --git a/bin/-tarball-info.js b/bin/-tarball-info.js index bfd14c2e0f4..9a1d8d4d876 100644 --- a/bin/-tarball-info.js +++ b/bin/-tarball-info.js @@ -166,7 +166,9 @@ function generatePackageReference(version, tarballName) { } function insertTarballsToPackageJson(fileLocation, options = {}) { - const pkgInfo = require(fileLocation); + const location = require.resolve(fileLocation); + delete require[location]; + const pkgInfo = require(location); if (options.isRelativeTarball) { pkgInfo.version = `${pkgInfo.version}-sha.${CurrentSha}`; } @@ -182,11 +184,11 @@ function insertTarballsToPackageJson(fileLocation, options = {}) { if (!options.isRelativeTarball) { const resolutions = (pkgInfo.resolutions = pkgInfo.resolutions || {}); - resolutions[packageName] = pkg.tarballLocation; + resolutions[packageName] = pkg.reference; } }); - fs.writeFileSync(fileLocation, JSON.stringify(pkgInfo, null, 2)); + fs.writeFileSync(location, JSON.stringify(pkgInfo, null, 2)); } module.exports = { diff --git a/bin/test-external-partner-project.js b/bin/test-external-partner-project.js index e7debce767e..54be70cd87a 100755 --- a/bin/test-external-partner-project.js +++ b/bin/test-external-partner-project.js @@ -9,15 +9,26 @@ const execa = require('execa'); const debug = require('debug')('test-external'); const rimraf = require('rimraf'); const chalk = require('chalk'); - +const cliArgs = require('command-line-args'); const projectRoot = path.resolve(__dirname, '../'); -const externalProjectName = process.argv[2]; -const gitUrl = process.argv[3]; -const skipSmokeTest = - (process.argv[4] && process.argv[4] === '--skip-smoke-test') || - (process.argv[5] && process.argv[5] === '--skip-smoke-test'); -const skipClone = - (process.argv[4] && process.argv[4] === '--skip-clone') || (process.argv[5] && process.argv[5] === '--skip-clone'); + +let cliOptionsDef = [{ name: 'projectName', defaultOption: true }]; +let cliOptions = cliArgs(cliOptionsDef, { stopAtFirstUnknown: true }); +const externalProjectName = cliOptions.projectName; +let argv = cliOptions._unknown || []; +cliOptionsDef = [{ name: 'gitUrl', defaultOption: true }]; +cliOptions = cliArgs(cliOptionsDef, { stopAtFirstUnknown: true, argv }); +const gitUrl = cliOptions.gitUrl; +argv = cliOptions._unknown || []; +cliOptionsDef = [ + { name: 'skipSmokeTest', type: Boolean, defaultValue: false }, + { name: 'skipClone', type: Boolean, defaultValue: false }, + { name: 'skipTest', type: Boolean, defaultValue: false }, +]; +cliOptions = cliArgs(cliOptionsDef, { argv }); + +const { skipSmokeTest, skipClone, skipTest } = cliOptions; + // we share this for the build const cachePath = '../__external-test-cache'; const tempDir = path.join(projectRoot, cachePath); @@ -109,34 +120,39 @@ try { try { debug('Preparing Package To Run Tests Against Commit'); insertTarballsToPackageJson(packageJsonLocation); - // we must use npm because yarn fails to install - // the nested tarballs correctly (just as it fails to pack them correctly) - // we rimraf node_modules first because it was previously - // installed using yarn's resolution algorithm - if (!skipSmokeTest) { - execExternal(`rm -rf node_modules`); - } - execExternal(`npm install`); + // lock files may bring in other versions of EmberData that otherwise + // would have resolved to our desired version + execExternal(`rm -rf node_modules package-lock.json yarn.lock`); + // we are forced to use yarn so that our resolutions will be respected + // in addition to the version file link we insert otherwise nested deps + // may bring their own ember-data + execExternal(`yarn install`); } catch (e) { debug(e); throw new Error(`Unable to npm install tarballs for ember-data\` for ${externalProjectName}`); } -try { - debug('Running tests against EmberData commit'); - execExternal(`ember test`, true); -} catch (e) { - commitTestPassed = false; +if (!skipTest) { + try { + debug('Running tests against EmberData commit'); + execExternal(`ember test`, true); + } catch (e) { + commitTestPassed = false; + } } -if (skipSmokeTest && !commitTestPassed) { - throw new Error('Commit may result in a regression, but the smoke test was skipped.'); -} else if (!smokeTestPassed && !commitTestPassed) { - throw new Error(`Commit may result in a regression, but the smoke test for ${externalProjectName} also failed.`); -} else if (smokeTestPassed && !commitTestPassed) { - throw new Error(`Commit results in a regression in ${externalProjectName}`); -} else if (!smokeTestPassed) { - console.log(`Commit may resolve issues present in the smoke test for ${externalProjectName}`); +if (skipTest) { + console.log(`Skipped Tests: Commit viability unknown`); } else { - console.log(`Commit does not regress ${externalProjectName}`); + if (skipSmokeTest && !commitTestPassed) { + throw new Error('Commit may result in a regression, but the smoke test was skipped.'); + } else if (!smokeTestPassed && !commitTestPassed) { + throw new Error(`Commit may result in a regression, but the smoke test for ${externalProjectName} also failed.`); + } else if (smokeTestPassed && !commitTestPassed) { + throw new Error(`Commit results in a regression in ${externalProjectName}`); + } else if (!smokeTestPassed) { + console.log(`Commit may resolve issues present in the smoke test for ${externalProjectName}`); + } else { + console.log(`Commit does not regress ${externalProjectName}`); + } } From bea016cb16f5dcfc6f960080980bc1abb13f7782 Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Mon, 25 Nov 2019 23:25:22 -0800 Subject: [PATCH 2/5] add log to figure out CI and update ilios --- bin/test-external-partner-project.js | 5 +++-- package.json | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/bin/test-external-partner-project.js b/bin/test-external-partner-project.js index 54be70cd87a..a94216cab57 100755 --- a/bin/test-external-partner-project.js +++ b/bin/test-external-partner-project.js @@ -128,8 +128,9 @@ try { // may bring their own ember-data execExternal(`yarn install`); } catch (e) { - debug(e); - throw new Error(`Unable to npm install tarballs for ember-data\` for ${externalProjectName}`); + console.log(`Unable to npm install tarballs for ember-data\` for ${externalProjectName}. Original error below:`); + + throw e; } if (!skipTest) { diff --git a/package.json b/package.json index d5dfda3d828..a93be039aaf 100644 --- a/package.json +++ b/package.json @@ -32,7 +32,7 @@ "test-external:travis-web": "./bin/test-external-partner-project.js travis-web https://github.com/travis-ci/travis-web.git", "test-external:storefront": "./bin/test-external-partner-project.js storefront https://github.com/embermap/ember-data-storefront.git", "test-external:factory-guy": "./bin/test-external-partner-project.js factory-guy https://github.com/danielspaniel/ember-data-factory-guy.git", - "test-external:ilios-frontend": "./bin/test-external-partner-project.js ilios-frontend https://github.com/ilios/frontend.git --skip-smoke-test", + "test-external:ilios-frontend": "./bin/test-external-partner-project.js ilios-frontend https://github.com/ilios/frontend.git --skipSmokeTest", "test-external:ember-resource-metadata": "./bin/test-external-partner-project.js ember-resource-metadata https://github.com/ef4/ember-resource-metadata.git", "test-external:ember-data-relationship-tracker": "./bin/test-external-partner-project.js ember-data-relationship-tracker https://github.com/ef4/ember-data-relationship-tracker.git" }, From 67dc107ef5d6bd6c0b211a48b70637f5a7c05525 Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Tue, 26 Nov 2019 00:07:31 -0800 Subject: [PATCH 3/5] make extra sure we aren't trolling ourselves --- bin/test-external-partner-project.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/bin/test-external-partner-project.js b/bin/test-external-partner-project.js index a94216cab57..51317cd4d59 100755 --- a/bin/test-external-partner-project.js +++ b/bin/test-external-partner-project.js @@ -120,13 +120,16 @@ try { try { debug('Preparing Package To Run Tests Against Commit'); insertTarballsToPackageJson(packageJsonLocation); - // lock files may bring in other versions of EmberData that otherwise - // would have resolved to our desired version - execExternal(`rm -rf node_modules package-lock.json yarn.lock`); + + // clear node_modules installed for the smoke-test + execExternal(`rm -rf node_modules`); // we are forced to use yarn so that our resolutions will be respected // in addition to the version file link we insert otherwise nested deps // may bring their own ember-data - execExternal(`yarn install`); + // + // For this reason we don't trust the lock file + // we also can't trust the cache + execExternal(`yarn install --no-lockfile --cache-folder=tmp/yarn-cache`); } catch (e) { console.log(`Unable to npm install tarballs for ember-data\` for ${externalProjectName}. Original error below:`); From 77e97f524c5b72bceb8e15717cd99de0c6150a4a Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Tue, 26 Nov 2019 13:15:11 -0800 Subject: [PATCH 4/5] cleanup --- bin/-tarball-info.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bin/-tarball-info.js b/bin/-tarball-info.js index 9a1d8d4d876..fc7169be3a1 100644 --- a/bin/-tarball-info.js +++ b/bin/-tarball-info.js @@ -166,8 +166,11 @@ function generatePackageReference(version, tarballName) { } function insertTarballsToPackageJson(fileLocation, options = {}) { + // in some flows we have the potential to have previously written + // to the package.json already prior to calling this method. + // this ensures we get the latest and not a stale module const location = require.resolve(fileLocation); - delete require[location]; + delete require.cache[location]; const pkgInfo = require(location); if (options.isRelativeTarball) { pkgInfo.version = `${pkgInfo.version}-sha.${CurrentSha}`; From 0952f7c6310dc747b0aa65bd59b1313c2e7e95fd Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Tue, 26 Nov 2019 13:36:10 -0800 Subject: [PATCH 5/5] improve read-file --- bin/-tarball-info.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/bin/-tarball-info.js b/bin/-tarball-info.js index fc7169be3a1..776f5351eec 100644 --- a/bin/-tarball-info.js +++ b/bin/-tarball-info.js @@ -168,10 +168,11 @@ function generatePackageReference(version, tarballName) { function insertTarballsToPackageJson(fileLocation, options = {}) { // in some flows we have the potential to have previously written // to the package.json already prior to calling this method. - // this ensures we get the latest and not a stale module + // reading it in this way this ensures we get the latest and not + // a stale module from require const location = require.resolve(fileLocation); - delete require.cache[location]; - const pkgInfo = require(location); + const pkgInfo = JSON.parse(fs.readFileSync(location, 'utf8')); + if (options.isRelativeTarball) { pkgInfo.version = `${pkgInfo.version}-sha.${CurrentSha}`; }