From e6987e7cd86a1fba36cbf32725c44525d4c1938a Mon Sep 17 00:00:00 2001 From: Dennis Snell Date: Wed, 26 Oct 2022 20:04:37 -0700 Subject: [PATCH] Perf Tests: Refactor use of ENV and git in test setup In this patch we're refactoring how the performance tests are configured, relying on the ENV values set by Github Actions and adding new semantic terms for existing implicit conditions. Additionally, the custom `git` abstraction has been removed and replaced with `SimpleGit` for a more concrete represetnation of what is being performed during the test setup. This change will enable more complicated uses of `git` for optimizing the run script, and by moving out of the abstraction it will minimize any changes based on that kind of `git` usage. There should be no functional changes in this patch and only the code surrounding test setup should change. The term "branch" is replaced in a few places with "ref" since this script is designed to accept any `git` ref pointing to commits, and in fact has been in daily use with a mix of tags, commit SHAs, and branch names. --- bin/plugin/commands/common.js | 60 +-------- bin/plugin/commands/packages.js | 154 +++++++++++++++------- bin/plugin/commands/performance.js | 91 +++++++------ bin/plugin/lib/git.js | 202 ----------------------------- bin/tsconfig.json | 1 - 5 files changed, 161 insertions(+), 347 deletions(-) delete mode 100644 bin/plugin/lib/git.js diff --git a/bin/plugin/commands/common.js b/bin/plugin/commands/common.js index fc540b073a1ff..59abfa93447ef 100644 --- a/bin/plugin/commands/common.js +++ b/bin/plugin/commands/common.js @@ -1,72 +1,26 @@ /** * External dependencies */ -const fs = require( 'fs' ); -const rimraf = require( 'rimraf' ); const semver = require( 'semver' ); +const SimpleGit = require( 'simple-git' ); /** * Internal dependencies */ -const { log, formats } = require( '../lib/logger' ); -const { runStep, readJSONFile } = require( '../lib/utils' ); -const git = require( '../lib/git' ); -const config = require( '../config' ); - -/** - * Clone the repository and returns the working directory. - * - * @param {string} abortMessage Abort message. - * - * @return {Promise} Repository local path. - */ -async function runGitRepositoryCloneStep( abortMessage ) { - // Cloning the repository. - let gitWorkingDirectoryPath; - await runStep( 'Cloning the Git repository', abortMessage, async () => { - log( '>> Cloning the Git repository' ); - gitWorkingDirectoryPath = await git.clone( config.gitRepositoryURL ); - log( - '>> The Git repository has been successfully cloned in the following temporary folder: ' + - formats.success( gitWorkingDirectoryPath ) - ); - } ); - - return gitWorkingDirectoryPath; -} - -/** - * Clean the working directories. - * - * @param {string[]} folders Folders to clean. - * @param {string} abortMessage Abort message. - */ -async function runCleanLocalFoldersStep( folders, abortMessage ) { - await runStep( 'Cleaning the temporary folders', abortMessage, async () => { - await Promise.all( - folders.map( async ( directoryPath ) => { - if ( fs.existsSync( directoryPath ) ) { - await rimraf( directoryPath, ( err ) => { - if ( err ) { - throw err; - } - } ); - } - } ) - ); - } ); -} +const { readJSONFile } = require( '../lib/utils' ); /** * Finds the name of the current plugin release branch based on the version in - * the package.json file. + * the package.json file and the latest `trunk` branch in `git`. * * @param {string} gitWorkingDirectoryPath Path to the project's working directory. * * @return {string} Name of the plugin release branch. */ async function findPluginReleaseBranchName( gitWorkingDirectoryPath ) { - await git.checkoutRemoteBranch( gitWorkingDirectoryPath, 'trunk' ); + await SimpleGit( gitWorkingDirectoryPath ) + .fetch( 'origin', 'trunk' ) + .checkout( 'trunk' ); const packageJsonPath = gitWorkingDirectoryPath + '/package.json'; const mainPackageJson = readJSONFile( packageJsonPath ); @@ -141,6 +95,4 @@ function calculateVersionBumpFromChangelog( module.exports = { calculateVersionBumpFromChangelog, findPluginReleaseBranchName, - runGitRepositoryCloneStep, - runCleanLocalFoldersStep, }; diff --git a/bin/plugin/commands/packages.js b/bin/plugin/commands/packages.js index e715ee69345ad..ba11010aa436d 100644 --- a/bin/plugin/commands/packages.js +++ b/bin/plugin/commands/packages.js @@ -6,20 +6,24 @@ const path = require( 'path' ); const glob = require( 'fast-glob' ); const fs = require( 'fs' ); const { inc: semverInc } = require( 'semver' ); +const rimraf = require( 'rimraf' ); const readline = require( 'readline' ); +const { default: git } = require( 'simple-git' ); /** * Internal dependencies */ const { log, formats } = require( '../lib/logger' ); -const { askForConfirmation, runStep, readJSONFile } = require( '../lib/utils' ); +const { + askForConfirmation, + runStep, + readJSONFile, + getRandomTemporaryPath, +} = require( '../lib/utils' ); const { calculateVersionBumpFromChangelog, findPluginReleaseBranchName, - runGitRepositoryCloneStep, - runCleanLocalFoldersStep, } = require( './common' ); -const git = require( '../lib/git' ); const { join } = require( 'path' ); /** @@ -55,6 +59,17 @@ const { join } = require( 'path' ); * @property {ReleaseType} releaseType The selected release type. */ +/** + * Throws if given an error in the node.js callback style. + * + * @param {any|null} error If callback failed, this will hold a value. + */ +const rethrow = ( error ) => { + if ( error ) { + throw error; + } +}; + /** * Checks out the npm release branch. * @@ -64,9 +79,28 @@ async function checkoutNpmReleaseBranch( { gitWorkingDirectoryPath, npmReleaseBranch, } ) { - // Creating the release branch. - await git.checkoutRemoteBranch( gitWorkingDirectoryPath, npmReleaseBranch ); - await git.fetch( gitWorkingDirectoryPath, [ '--depth=100' ] ); + /* + * Create the release branch. + * + * Note that we are grabbing an arbitrary depth of commits + * during the fetch. When `lerna` attempts to determine if + * a package needs an update, it looks at `git` history, + * and if we have pruned that history it will pre-emptively + * publish when it doesn't need to. + * + * We could set a different arbitrary depth if this isn't + * long enough or if it's excessive. We could also try and + * find a way to more specifically fetch what we expect to + * change. For example, if we knew we'll be performing + * updates every two weeks, we might be conservative and + * use `--shallow-since=4.weeks.ago`. + * + * At the time of writing, a depth of 100 pulls in all + * `trunk` commits from within the past week. + */ + await git( gitWorkingDirectoryPath ) + .fetch( npmReleaseBranch, [ '--depth=100' ] ) + .checkout( npmReleaseBranch ); log( '>> The local npm release branch ' + formats.success( npmReleaseBranch ) + @@ -105,13 +139,19 @@ async function runNpmReleaseBranchSyncStep( pluginReleaseBranch, config ) { `>> Syncing the latest plugin release to "${ pluginReleaseBranch }".` ); - await git.replaceContentFromRemoteBranch( - gitWorkingDirectoryPath, - pluginReleaseBranch - ); + const repo = git( gitWorkingDirectoryPath ); + + /* + * Replace content from remote branch. + * + * @TODO: What is our goal here? Could `git reset --hard origin/${pluginReleaseBranch}` work? + * Why are we manually removing and then adding files back in? + */ + await repo + .raw( 'rm', '-r', '.' ) + .raw( 'checkout', `origin/${ pluginReleaseBranch }`, '--', '.' ); - const commitHash = await git.commit( - gitWorkingDirectoryPath, + const { commit: commitHash } = await repo.commit( `Merge changes published in the Gutenberg plugin "${ pluginReleaseBranch }" branch` ); @@ -223,6 +263,7 @@ async function updatePackages( config ) { '>> Recommended version bumps based on the changes detected in CHANGELOG files:' ); + // e.g. "2022-11-01T00:13:26.102Z" -> "2022-11-01" const publishDate = new Date().toISOString().split( 'T' )[ 0 ]; await Promise.all( packagesToUpdate.map( @@ -234,11 +275,8 @@ async function updatePackages( config ) { version, } ) => { // Update changelog. - const content = await fs.promises.readFile( - changelogPath, - 'utf8' - ); - await fs.promises.writeFile( + const content = fs.readFileSync( changelogPath, 'utf8' ); + fs.writeFileSync( changelogPath, content.replace( '## Unreleased', @@ -280,11 +318,10 @@ async function updatePackages( config ) { ); } - const commitHash = await git.commit( - gitWorkingDirectoryPath, - 'Update changelog files', - [ './*' ] - ); + const { commit: commitHash } = await git( gitWorkingDirectoryPath ) + .add( [ './*' ] ) + .commit( 'Update changelog files' ); + if ( commitHash ) { await runPushGitChangesStep( config ); } @@ -313,10 +350,7 @@ async function runPushGitChangesStep( { abortMessage ); } - await git.pushBranchToOrigin( - gitWorkingDirectoryPath, - npmReleaseBranch - ); + await git( gitWorkingDirectoryPath ).push( 'origin', npmReleaseBranch ); } ); } @@ -345,9 +379,10 @@ async function publishPackagesToNpm( { stdio: 'inherit', } ); - const beforeCommitHash = await git.getLastCommitHash( - gitWorkingDirectoryPath - ); + const beforeCommitHash = await git( gitWorkingDirectoryPath ).revparse( [ + '--short', + 'HEAD', + ] ); const yesFlag = interactive ? '' : '--yes'; const noVerifyAccessFlag = interactive ? '' : '--no-verify-access'; @@ -403,9 +438,10 @@ async function publishPackagesToNpm( { ); } - const afterCommitHash = await git.getLastCommitHash( - gitWorkingDirectoryPath - ); + const afterCommitHash = await git( gitWorkingDirectoryPath ).revparse( [ + '--short', + 'HEAD', + ] ); if ( afterCommitHash === beforeCommitHash ) { return; } @@ -439,18 +475,22 @@ async function backportCommitsToBranch( log( `>> Backporting commits to "${ branchName }".` ); - await git.resetLocalBranchAgainstOrigin( - gitWorkingDirectoryPath, - branchName - ); + const repo = git( gitWorkingDirectoryPath ); + + /* + * Reset any local changes and replace them with the origin branch's copy. + * + * @TODO: Why do we use fetch/checkout/pull here instead of checkout/reset--hard? + * Is the fetch necessary at all? Do we expect the remote branch to be + * different now than when we started running this script? + */ + await repo.fetch().checkout( branchName ).pull( 'origin', branchName ); + for ( const commitHash of commits ) { - await git.cherrypickCommitIntoBranch( - gitWorkingDirectoryPath, - branchName, - commitHash - ); + await repo.raw( 'cherry-pick', commitHash ); } - await git.pushBranchToOrigin( gitWorkingDirectoryPath, branchName ); + + await repo.push( 'origin', branchName ); log( `>> Backporting successfully finished.` ); } @@ -478,11 +518,20 @@ async function runPackagesRelease( config, customMessages ) { const temporaryFolders = []; if ( ! config.gitWorkingDirectoryPath ) { - // Cloning the Git repository. - config.gitWorkingDirectoryPath = await runGitRepositoryCloneStep( - config.abortMessage + const gitPath = getRandomTemporaryPath(); + config.gitWorkingDirectoryPath = gitPath; + fs.mkdirSync( gitPath, { recursive: true } ); + temporaryFolders.push( gitPath ); + + await runStep( + 'Cloning the Git repository', + config.abortMessage, + async () => { + log( '>> Cloning the Git repository' ); + await git( gitPath ).clone( config.gitRepositoryURL ); + log( ` >> successfully clone into: ${ gitPath }` ); + } ); - temporaryFolders.push( config.gitWorkingDirectoryPath ); } let pluginReleaseBranch; @@ -518,7 +567,16 @@ async function runPackagesRelease( config, customMessages ) { } } - await runCleanLocalFoldersStep( temporaryFolders, 'Cleaning failed.' ); + await runStep( + 'Cleaning the temporary folders', + 'Cleaning failed', + async () => + await Promise.all( + temporaryFolders + .filter( ( tempDir ) => fs.existsSync( tempDir ) ) + .map( ( tempDir ) => rimraf( tempDir, rethrow ) ) + ) + ); log( '\n>> 🎉 WordPress packages are now published!\n\n', diff --git a/bin/plugin/commands/performance.js b/bin/plugin/commands/performance.js index 0571a0e72955f..2aa8a2c9a7f5c 100644 --- a/bin/plugin/commands/performance.js +++ b/bin/plugin/commands/performance.js @@ -4,6 +4,7 @@ const fs = require( 'fs' ); const path = require( 'path' ); const { mapValues, kebabCase } = require( 'lodash' ); +const SimpleGit = require( 'simple-git' ); /** * Internal dependencies @@ -15,7 +16,6 @@ const { askForConfirmation, getRandomTemporaryPath, } = require( '../lib/utils' ); -const git = require( '../lib/git' ); const config = require( '../config' ); /** @@ -146,24 +146,6 @@ function curateResults( results ) { }; } -/** - * Set up the given branch for testing. - * - * @param {string} branch Branch name. - * @param {string} environmentDirectory Path to the plugin environment's clone. - */ -async function setUpGitBranch( branch, environmentDirectory ) { - // Restore clean working directory (e.g. if `package-lock.json` has local - // changes after install). - await git.discardLocalChanges( environmentDirectory ); - - log( ' >> Fetching the ' + formats.success( branch ) + ' branch' ); - await git.checkoutRemoteBranch( environmentDirectory, branch ); - - log( ' >> Building the ' + formats.success( branch ) + ' branch' ); - await runShellScript( 'npm ci && npm run build', environmentDirectory ); -} - /** * Runs the performance tests on the current branch. * @@ -214,24 +196,47 @@ async function runPerformanceTests( branches, options ) { // 1- Preparing the tests directory. log( '\n>> Preparing the tests directories' ); log( ' >> Cloning the repository' ); - const baseDirectory = await git.clone( config.gitRepositoryURL ); + + /** + * @type {string[]} git refs against which to run tests; + * could be commit SHA, branch name, tag, etc... + */ + if ( branches.length < 2 ) { + throw new Error( `Need at least two git refs to run` ); + } + + const baseDirectory = getRandomTemporaryPath(); + fs.mkdirSync( baseDirectory, { recursive: true } ); + + // @ts-ignore + const git = SimpleGit( baseDirectory ); + await git + .raw( 'init' ) + .raw( 'remote', 'add', 'origin', config.gitRepositoryURL ); + + for ( const branch of branches ) { + await git.raw( 'fetch', '--depth=1', 'origin', branch ); + } + + await git.raw( 'checkout', branches[ 0 ] ); + const rootDirectory = getRandomTemporaryPath(); const performanceTestDirectory = rootDirectory + '/tests'; await runShellScript( 'mkdir -p ' + rootDirectory ); await runShellScript( 'cp -R ' + baseDirectory + ' ' + performanceTestDirectory ); + if ( !! options.testsBranch ) { - log( - ' >> Fetching the test branch: ' + - formats.success( options.testsBranch ) + - ' branch' - ); - await git.checkoutRemoteBranch( - performanceTestDirectory, - options.testsBranch - ); + const branchName = formats.success( options.testsBranch ); + log( ` >> Fetching the test-runner branch: ${ branchName }` ); + + // @ts-ignore + await SimpleGit( performanceTestDirectory ) + .raw( 'fetch', '--depth=1', 'origin', options.testsBranch ) + .raw( 'checkout', options.testsBranch ); } + log( ' >> Installing dependencies and building packages' ); await runShellScript( 'npm ci && npm run build:packages', @@ -244,16 +249,22 @@ async function runPerformanceTests( branches, options ) { log( '\n>> Preparing an environment directory per branch' ); const branchDirectories = {}; for ( const branch of branches ) { - log( ' >> Branch: ' + branch ); + log( ` >> Branch: ${ branch }` ); const environmentDirectory = rootDirectory + '/envs/' + kebabCase( branch ); // @ts-ignore branchDirectories[ branch ] = environmentDirectory; + const buildPath = `${ environmentDirectory }/plugin`; await runShellScript( 'mkdir ' + environmentDirectory ); - await runShellScript( - 'cp -R ' + baseDirectory + ' ' + environmentDirectory + '/plugin' - ); - await setUpGitBranch( branch, environmentDirectory + '/plugin' ); + await runShellScript( `cp -R ${ baseDirectory } ${ buildPath }` ); + + log( ` >> Fetching the ${ formats.success( branch ) } branch` ); + // @ts-ignore + await SimpleGit( buildPath ).reset( 'hard' ).checkout( branch ); + + log( ` >> Building the ${ formats.success( branch ) } branch` ); + await runShellScript( 'npm ci && npm run build', buildPath ); + await runShellScript( 'cp ' + path.resolve( @@ -302,13 +313,9 @@ async function runPerformanceTests( branches, options ) { formats.success( performanceTestDirectory ) ); for ( const branch of branches ) { - log( - '>> Environment Directory (' + - branch + - ') : ' + - // @ts-ignore - formats.success( branchDirectories[ branch ] ) - ); + // @ts-ignore + const envPath = formats.success( branchDirectories[ branch ] ); + log( `>> Environment Directory (${ branch }) : ${ envPath }` ); } // 4- Running the tests. @@ -328,7 +335,7 @@ async function runPerformanceTests( branches, options ) { for ( const branch of branches ) { // @ts-ignore const environmentDirectory = branchDirectories[ branch ]; - log( ' >> Branch: ' + branch + ', Suite: ' + testSuite ); + log( ` >> Branch: ${ branch }, Suite: ${ testSuite }` ); log( ' >> Starting the environment.' ); await runShellScript( '../../tests/node_modules/.bin/wp-env start', diff --git a/bin/plugin/lib/git.js b/bin/plugin/lib/git.js deleted file mode 100644 index 8208efa7164d1..0000000000000 --- a/bin/plugin/lib/git.js +++ /dev/null @@ -1,202 +0,0 @@ -// @ts-nocheck -/** - * External dependencies - */ -const SimpleGit = require( 'simple-git' ); - -/** - * Internal dependencies - */ -const { getRandomTemporaryPath } = require( './utils' ); - -/** - * Clones a GitHub repository. - * - * @param {string} repositoryUrl - * - * @return {Promise} Repository local Path - */ -async function clone( repositoryUrl ) { - const gitWorkingDirectoryPath = getRandomTemporaryPath(); - const simpleGit = SimpleGit(); - await simpleGit.clone( repositoryUrl, gitWorkingDirectoryPath, [ - '--depth=1', - '--no-single-branch', - ] ); - return gitWorkingDirectoryPath; -} - -/** - * Fetches changes from the repository. - * - * @param {string} gitWorkingDirectoryPath Local repository path. - * @param {string[]|Object} options Git options to apply. - */ -async function fetch( gitWorkingDirectoryPath, options = [] ) { - const simpleGit = SimpleGit( gitWorkingDirectoryPath ); - await simpleGit.fetch( options ); -} - -/** - * Commits changes to the repository. - * - * @param {string} gitWorkingDirectoryPath Local repository path. - * @param {string} message Commit message. - * @param {string[]} filesToAdd Files to add. - * - * @return {Promise} Commit Hash - */ -async function commit( gitWorkingDirectoryPath, message, filesToAdd = [] ) { - const simpleGit = SimpleGit( gitWorkingDirectoryPath ); - await simpleGit.add( filesToAdd ); - const commitData = await simpleGit.commit( message ); - const commitHash = commitData.commit; - - return commitHash; -} - -/** - * Creates a local branch. - * - * @param {string} gitWorkingDirectoryPath Local repository path. - * @param {string} branchName Branch Name - */ -async function createLocalBranch( gitWorkingDirectoryPath, branchName ) { - const simpleGit = SimpleGit( gitWorkingDirectoryPath ); - await simpleGit.checkoutLocalBranch( branchName ); -} - -/** - * Checkout a local branch. - * - * @param {string} gitWorkingDirectoryPath Local repository path. - * @param {string} branchName Branch Name - */ -async function checkoutRemoteBranch( gitWorkingDirectoryPath, branchName ) { - const simpleGit = SimpleGit( gitWorkingDirectoryPath ); - await simpleGit.fetch( 'origin', branchName ); - await simpleGit.checkout( branchName ); -} - -/** - * Creates a local tag. - * - * @param {string} gitWorkingDirectoryPath Local repository path. - * @param {string} tagName Tag Name - */ -async function createLocalTag( gitWorkingDirectoryPath, tagName ) { - const simpleGit = SimpleGit( gitWorkingDirectoryPath ); - await simpleGit.addTag( tagName ); -} - -/** - * Pushes a local branch to the origin. - * - * @param {string} gitWorkingDirectoryPath Local repository path. - * @param {string} branchName Branch Name - */ -async function pushBranchToOrigin( gitWorkingDirectoryPath, branchName ) { - const simpleGit = SimpleGit( gitWorkingDirectoryPath ); - await simpleGit.push( 'origin', branchName ); -} - -/** - * Pushes tags to the origin. - * - * @param {string} gitWorkingDirectoryPath Local repository path. - */ -async function pushTagsToOrigin( gitWorkingDirectoryPath ) { - const simpleGit = SimpleGit( gitWorkingDirectoryPath ); - await simpleGit.pushTags( 'origin' ); -} - -/** - * Discard local changes. - * - * @param {string} gitWorkingDirectoryPath Local repository path. - */ -async function discardLocalChanges( gitWorkingDirectoryPath ) { - const simpleGit = SimpleGit( gitWorkingDirectoryPath ); - await simpleGit.reset( 'hard' ); -} - -/** - * Reset local branch against the origin. - * - * @param {string} gitWorkingDirectoryPath Local repository path. - * @param {string} branchName Branch Name - */ -async function resetLocalBranchAgainstOrigin( - gitWorkingDirectoryPath, - branchName -) { - const simpleGit = SimpleGit( gitWorkingDirectoryPath ); - await simpleGit.fetch(); - await simpleGit.checkout( branchName ); - await simpleGit.pull( 'origin', branchName ); -} - -/** - * Gets the commit hash for the last commit in the current branch. - * - * @param {string} gitWorkingDirectoryPath Local repository path. - * - * @return {string} Commit hash. - */ -async function getLastCommitHash( gitWorkingDirectoryPath ) { - const simpleGit = SimpleGit( gitWorkingDirectoryPath ); - return await simpleGit.revparse( [ '--short', 'HEAD' ] ); -} - -/** - * Cherry-picks a commit into trunk - * - * @param {string} gitWorkingDirectoryPath Local repository path. - * @param {string} branchName Branch name. - * @param {string} commitHash Commit hash. - */ -async function cherrypickCommitIntoBranch( - gitWorkingDirectoryPath, - branchName, - commitHash -) { - const simpleGit = SimpleGit( gitWorkingDirectoryPath ); - await simpleGit.checkout( branchName ); - await simpleGit.raw( [ 'cherry-pick', commitHash ] ); -} - -/** - * Replaces the local branch's content with the content from another branch. - * - * @param {string} gitWorkingDirectoryPath Local repository path. - * @param {string} sourceBranchName Branch Name - */ -async function replaceContentFromRemoteBranch( - gitWorkingDirectoryPath, - sourceBranchName -) { - const simpleGit = SimpleGit( gitWorkingDirectoryPath ); - await simpleGit.raw( [ 'rm', '-r', '.' ] ); - await simpleGit.raw( [ - 'checkout', - `origin/${ sourceBranchName }`, - '--', - '.', - ] ); -} - -module.exports = { - clone, - commit, - checkoutRemoteBranch, - createLocalBranch, - createLocalTag, - fetch, - pushBranchToOrigin, - pushTagsToOrigin, - discardLocalChanges, - resetLocalBranchAgainstOrigin, - getLastCommitHash, - cherrypickCommitIntoBranch, - replaceContentFromRemoteBranch, -}; diff --git a/bin/tsconfig.json b/bin/tsconfig.json index 86d2a07c742f8..f3d576c178d0c 100644 --- a/bin/tsconfig.json +++ b/bin/tsconfig.json @@ -25,7 +25,6 @@ "./plugin/lib/version.js", "./plugin/lib/logger.js", "./plugin/lib/utils.js", - "./plugin/lib/git.js", "./validate-package-lock.js" ] }