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

Add retrying CI test support #17219

Merged
merged 3 commits into from
Oct 4, 2024
Merged

Add retrying CI test support #17219

merged 3 commits into from
Oct 4, 2024

Conversation

Mati365
Copy link
Member

@Mati365 Mati365 commented Oct 4, 2024

Suggested merge commit message (convention)

Internal: Added the retrying test execution mechanism on CI. Closes #17169


Additional information

Added --retries process argv argument to specify how many times tests should be retries. It's 3 by default.

@Mati365 Mati365 requested a review from arkflpc October 4, 2024 05:47
@pomek
Copy link
Member

pomek commented Oct 4, 2024

I would implement it a bit differently:

diff --git a/scripts/ci/check-unit-tests-for-package.mjs b/scripts/ci/check-unit-tests-for-package.mjs
index 2833f127e9..c43895c245 100644
--- a/scripts/ci/check-unit-tests-for-package.mjs
+++ b/scripts/ci/check-unit-tests-for-package.mjs
@@ -21,9 +21,9 @@ main()
 	} );
 
 async function main() {
-	const { packageName, checkCoverage, allowNonFullCoverage, coverageFile, retries } = getOptions( process.argv.slice( 2 ) );
+	const { packageName, checkCoverage, allowNonFullCoverage, coverageFile, attempts } = getOptions( process.argv.slice( 2 ) );
 
-	runTests( { packageName, checkCoverage, retries } );
+	runTests( { packageName, checkCoverage, attempts } );
 
 	if ( checkCoverage && !allowNonFullCoverage ) {
 		const exitCode = checkCodeCoverage();
@@ -46,9 +46,9 @@ async function main() {
  * @param {Object} options
  * @param {String} options.packageName
  * @param {Boolean} options.checkCoverage
- * @param {Number} options.retries
+ * @param {Number} options.attempts
  */
-function runTests( { packageName, checkCoverage, retries = 3 } ) {
+function runTests( { packageName, checkCoverage, attempts = 3 } ) {
 	const shortName = packageName.replace( /^ckeditor5?-/, '' );
 
 	const testCommand = [
@@ -60,22 +60,23 @@ function runTests( { packageName, checkCoverage, retries = 3 } ) {
 		checkCoverage ? '--coverage' : null
 	].filter( Boolean );
 
-	for ( let retry = 0; retry < retries; retry++ ) {
-		try {
-			execSync( testCommand.join( ' ' ), {
-				cwd: CKEDITOR5_ROOT_PATH,
-				stdio: 'inherit'
-			} );
-
-			break;
-		} catch ( err ) {
-			if ( retry === retries ) {
-				throw err;
-			} else {
-				console.error( err );
-				console.log( `\n⚠️ Retry ${ retry + 1 } of ${ retries } for ${ packageName }!` );
-			}
+	try {
+		execSync( testCommand.join( ' ' ), {
+			cwd: CKEDITOR5_ROOT_PATH,
+			stdio: 'inherit'
+		} );
+	} catch ( err ) {
+		if ( !attempts ) {
+			throw err;
 		}
+
+		console.log( `\n⚠️ Retry the test execution. Remaining attempts: ${ attempts - 1 }.` );
+
+		return runTests({
+			packageName,
+			checkCoverage,
+			attempts: attempts - 1
+		})
 	}
 }
 
@@ -104,13 +105,14 @@ function checkCodeCoverage() {
  * @returns {Boolean} options.checkCoverage
  * @returns {Boolean} options.allowNonFullCoverage
  * @returns {String|null} options.coverageFile
- * @returns {Number} options.retries
+ * @returns {Number} options.attempts
  */
 function getOptions( argv ) {
 	const options = minimist( argv, {
 		string: [
 			'package-name',
-			'coverage-file'
+			'coverage-file',
+			'attempts'
 		],
 		boolean: [
 			'check-coverage',
@@ -122,7 +124,7 @@ function getOptions( argv ) {
 		}
 	} );
 
-	options.retries = Number( options.retries ?? 3 );
+	options.attempts = Number( options.attempts || 1 );
 	options.packageName = options[ 'package-name' ];
 	options.coverageFile = options[ 'coverage-file' ];
 	options.checkCoverage = options[ 'check-coverage' ];
diff --git a/scripts/ci/generate-circleci-configuration.mjs b/scripts/ci/generate-circleci-configuration.mjs
index dbad491dd9..6914d20954 100755
--- a/scripts/ci/generate-circleci-configuration.mjs
+++ b/scripts/ci/generate-circleci-configuration.mjs
@@ -235,7 +235,7 @@ function generateTestSteps( packages, { checkCoverage, coverageFile = null } ) {
 		const testCommand = [
 			'node',
 			'scripts/ci/check-unit-tests-for-package.mjs',
-			'--retries 3',
+			'--attempts 3',
 			'--package-name',
 			packageName,
 			checkCoverage ? '--check-coverage' : null,

How does it work?

image

image

WDYT?

@Mati365
Copy link
Member Author

Mati365 commented Oct 4, 2024

Looks cool! Thanks.

Copy link
Contributor

@psmyrek psmyrek left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

@pomek pomek merged commit 683a4cf into master Oct 4, 2024
9 checks passed
@pomek pomek deleted the ck/17169 branch October 4, 2024 08:22
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.

Add multiple attends to run package test suite
3 participants