Skip to content

Commit

Permalink
Perf Tests: Refactor use of ENV and git in test setup
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@dmsnell
dmsnell committed Nov 1, 2022
1 parent 62987d7 commit e6987e7
Show file tree
Hide file tree
Showing 5 changed files with 161 additions and 347 deletions.
60 changes: 6 additions & 54 deletions bin/plugin/commands/common.js
View file
Original file line number Diff line number Diff line change
@@ -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<string>} 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 );
Expand Down Expand Up @@ -141,6 +95,4 @@ function calculateVersionBumpFromChangelog(
module.exports = {
calculateVersionBumpFromChangelog,
findPluginReleaseBranchName,
runGitRepositoryCloneStep,
runCleanLocalFoldersStep,
};
154 changes: 106 additions & 48 deletions bin/plugin/commands/packages.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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' );

/**
Expand Down Expand Up @@ -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.
*
Expand All @@ -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 ) +
Expand Down Expand Up @@ -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`
);

Expand Down Expand Up @@ -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(
Expand All @@ -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',
Expand Down Expand Up @@ -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 );
}
Expand Down Expand Up @@ -313,10 +350,7 @@ async function runPushGitChangesStep( {
abortMessage
);
}
await git.pushBranchToOrigin(
gitWorkingDirectoryPath,
npmReleaseBranch
);
await git( gitWorkingDirectoryPath ).push( 'origin', npmReleaseBranch );
} );
}

Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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.` );
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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',
Expand Down
Loading

0 comments on commit e6987e7

Please sign in to comment.