Properly group results by URL (#229)

swissspidy · Jan 8, 2025 · 4213bd1 · 4213bd1
1 parent 3243b29
commit 4213bd1
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 55 deletions.
diff --git a/action.yml b/action.yml
@@ -104,15 +104,15 @@ runs:
     - name: Install dependencies
       run: |
         echo "::group::Install dependencies"
-        npm ci ${{ inputs.debug != 'true' && '--silent' }}
+        npm ci ${{ inputs.debug != 'true' && '--silent' || '' }}
         echo "::endgroup::"
       shell: 'bash'
       working-directory: ${{ github.action_path }}/env
 
     - name: Install Playwright browsers
       run: |
         echo "::group::Install Playwright browsers"
-        npx ${{ inputs.debug != 'true' && '--silent' }} playwright install --with-deps
+        npx ${{ inputs.debug != 'true' && '--silent' || '' }} playwright install --with-deps
         echo "::endgroup::"
       if: ${{ inputs.action == 'test' }}
       shell: 'bash'
@@ -179,14 +179,12 @@ runs:
         ARGS+=(--wp=$WP_VERSION)
         ARGS+=(--php=$PHP_VERSION)
 
-        echo "::group::Start Playground server"
-
         IFS=,
         echo "Providing arguments: ${ARGS[*]}"
         unset IFS;
 
+        echo "Start Playground server..."
         ./node_modules/@wp-playground/cli/wp-playground.js server "${ARGS[@]}" &
-        echo "::endgroup::"
       env:
         PLUGINS: ${{ inputs.plugins }}
         THEMES: ${{ inputs.themes }}
@@ -197,7 +195,7 @@ runs:
       working-directory: ${{ github.action_path }}/env
 
     - name: Run tests
-      run: npm run ${{ inputs.debug != 'true' && '--silent' }} test:performance $ADDITIONAL_ARGS
+      run: npm run ${{ inputs.debug != 'true' && '--silent' || '' }} test:performance $ADDITIONAL_ARGS
       if: ${{ inputs.action == 'test' }}
       env:
         WP_BASE_URL: 'http://127.0.0.1:9400'
@@ -213,7 +211,7 @@ runs:
       working-directory: ${{ github.action_path }}/env
 
     - name: Stop server
-      run: npm run stop-server
+      run: npm run ${{ inputs.debug != 'true' && '--silent' || '' }} stop-server
       shell: 'bash'
       working-directory: ${{ github.action_path }}/env
 
@@ -226,7 +224,7 @@ runs:
         merge-multiple: true
 
     - name: Merge into single performance report
-      run: npm run ${{ inputs.debug != 'true' && '--silent' }} test:performance:merge-reports ${{ steps.download.outputs.download-path }}
+      run: npm run ${{ inputs.debug != 'true' && '--silent' || '' }} test:performance:merge-reports ${{ steps.download.outputs.download-path }}
       if: ${{ inputs.action == 'merge' }}
       shell: 'bash'
       working-directory: ${{ github.action_path }}/env
@@ -245,9 +243,9 @@ runs:
     - name: Log results
       run: |
         if [ ! -z $PREVIOUS_RESULTS ] && [ -f $PREVIOUS_RESULTS ]; then
-          npm run ${{ inputs.debug != 'true' && '--silent' }} test:performance:results $RESULTS_FILE $PREVIOUS_RESULTS
+          npm run ${{ inputs.debug != 'true' && '--silent' || '' }} test:performance:results $RESULTS_FILE $PREVIOUS_RESULTS
         else
-          npm run ${{ inputs.debug != 'true' && '--silent' }} test:performance:results $RESULTS_FILE
+          npm run ${{ inputs.debug != 'true' && '--silent' || '' }} test:performance:results $RESULTS_FILE
         fi;
       if: ${{ inputs.action == 'test' || inputs.action == 'merge' }}
       env:

diff --git a/env/tests/performance/cli/results.js b/env/tests/performance/cli/results.js
@@ -90,6 +90,8 @@ function formatAsMarkdownTable( rows ) {
 /**
  * Computes the median number from an array numbers.
  *
+ * @todo Import this from utils/index.ts once this file is converted to TS.
+ *
  * @param {number[]} array List of numbers.
  * @return {number} Median.
  */
@@ -105,12 +107,12 @@ function median( array ) {
 }
 
 /**
- * @type {Array<{file: string, title: string, results: Record<string,number[]>[]}>}
+ * @type {Record< string, Array< Record< string, number[] > > >}
  */
-let beforeStats = [];
+let beforeStats = {};
 
 /**
- * @type {Array<{file: string, title: string, results: Record<string,number[]>[]}>}
+ * @type {Record< string, Array< Record< string, number[] > > >}
  */
 let afterStats;
 
@@ -199,11 +201,11 @@ function formatValue( value, key ) {
 	return `${ value.toFixed( 2 ) } ms`;
 }
 
-for ( const { file, title, results } of afterStats ) {
-	const prevStat = beforeStats.find( ( s ) => s.file === file );
+for ( const [ url, results ] of Object.entries( afterStats ) ) {
+	const prevStat = beforeStats[ url ];
 
 	/**
-	 * @type {Array<Record<string,string|number|boolean>>}
+	 * @type {Array< Record< string, string | number | boolean > >}
 	 */
 	const diffResults = [];
 
@@ -220,8 +222,8 @@ for ( const { file, title, results } of afterStats ) {
 		for ( const [ key, values ] of Object.entries( newResult ) ) {
 			// Only do comparison if the number of results is the same.
 			const prevValues =
-				prevStat?.results.length === results.length
-					? prevStat?.results[ i ][ key ]
+				prevStat && prevStat.length === results.length
+					? prevStat[ i ][ key ]
 					: null;
 
 			const value = median( values );
@@ -258,10 +260,10 @@ for ( const { file, title, results } of afterStats ) {
 		diffResults.push( diffResult );
 	}
 
-	console.log( title );
+	console.log( `URL: \`${ url }\`` );
 	console.table( diffResults );
 
-	summaryMarkdown += `**${ title }**\n\n`;
+	summaryMarkdown += `**URL: \`${ url }\`**\n\n`;
 	summaryMarkdown += `${ formatAsMarkdownTable( diffResults ) }\n`;
 }
 

diff --git a/env/tests/performance/config/performance-reporter.ts b/env/tests/performance/config/performance-reporter.ts
@@ -14,13 +14,7 @@ process.env.WP_ARTIFACTS_PATH ??= join( process.cwd(), 'artifacts' );
 class PerformanceReporter implements Reporter {
 	private shard?: FullConfig[ 'shard' ];
 
-	allResults: Record<
-		string,
-		{
-			title: string;
-			results: Record< string, number[] >[];
-		}
-	> = {};
+	allResults: Record< string, Array< Record< string, number[] > > > = {};
 
 	onBegin( config: FullConfig ) {
 		if ( config.shard ) {
@@ -42,14 +36,15 @@ class PerformanceReporter implements Reporter {
 		);
 
 		if ( performanceResults?.body ) {
-			this.allResults[ test.location.file ] ??= {
-				// 0 = empty, 1 = browser, 2 = file name, 3 = test suite name.
-				title: test.titlePath()[ 3 ],
-				results: [],
-			};
-			this.allResults[ test.location.file ].results.push(
-				JSON.parse( performanceResults.body.toString( 'utf-8' ) )
-			);
+			const resultsByUrl = JSON.parse( performanceResults.body.toString( 'utf-8' ) ) as Record< string, Record< string, number[] > >;
+
+			for ( const [url, results ] of Object.entries(resultsByUrl)) {
+				this.allResults[ url ] ??= [];
+
+				this.allResults[ url ].push(
+					results
+				);
+			}
 		}
 	}
 
@@ -62,8 +57,6 @@ class PerformanceReporter implements Reporter {
 	 * @param result
 	 */
 	onEnd( result: FullResult ) {
-		const summary = [];
-
 		if ( Object.keys( this.allResults ).length > 0 ) {
 			if ( this.shard ) {
 				console.log(
@@ -75,10 +68,10 @@ class PerformanceReporter implements Reporter {
 			console.log( `Status: ${ result.status }` );
 		}
 
-		for ( const [ file, { title, results } ] of Object.entries(
+		for ( const [ url, results ] of Object.entries(
 			this.allResults
 		) ) {
-			console.log( `\n${ title }\n` );
+			console.log( `\nURL: \`${ url }\`\n` );
 			console.table(
 				results.map( ( r ) =>
 					Object.fromEntries(
@@ -89,12 +82,6 @@ class PerformanceReporter implements Reporter {
 					)
 				)
 			);
-
-			summary.push( {
-				file,
-				title,
-				results,
-			} );
 		}
 
 		if ( ! existsSync( process.env.WP_ARTIFACTS_PATH as string ) ) {
@@ -106,7 +93,7 @@ class PerformanceReporter implements Reporter {
 				process.env.WP_ARTIFACTS_PATH as string,
 				'performance-results.json'
 			),
-			JSON.stringify( summary, null, 2 )
+			JSON.stringify( this.allResults, null, 2 )
 		);
 	}
 }

diff --git a/env/tests/performance/specs/main.spec.ts b/env/tests/performance/specs/main.spec.ts
@@ -1,7 +1,7 @@
 import { test } from '@wordpress/e2e-test-utils-playwright';
 import { camelCaseDashes } from '../utils';
 
-const results: Record< string, number[] > = {};
+const results: Record< string, Record< string, number[] > > = {};
 
 test.describe( 'Tests', () => {
 	test.use( {
@@ -43,20 +43,22 @@ test.describe( 'Tests', () => {
 
 				const serverTiming = await metrics.getServerTiming();
 
+				results[url] ??= {};
+
 				for ( const [ key, value ] of Object.entries( serverTiming ) ) {
-					results[ camelCaseDashes( key ) ] ??= [];
-					results[ camelCaseDashes( key ) ].push( value );
+					results[url][ camelCaseDashes( key ) ] ??= [];
+					results[url][ camelCaseDashes( key ) ].push( value );
 				}
 
 				const ttfb = await metrics.getTimeToFirstByte();
 				const lcp = await metrics.getLargestContentfulPaint();
 
-				results.largestContentfulPaint ??= [];
-				results.largestContentfulPaint.push( lcp );
-				results.timeToFirstByte ??= [];
-				results.timeToFirstByte.push( ttfb );
-				results.lcpMinusTtfb ??= [];
-				results.lcpMinusTtfb.push( lcp - ttfb );
+				results[url].largestContentfulPaint ??= [];
+				results[url].largestContentfulPaint.push( lcp );
+				results[url].timeToFirstByte ??= [];
+				results[url].timeToFirstByte.push( ttfb );
+				results[url].lcpMinusTtfb ??= [];
+				results[url].lcpMinusTtfb.push( lcp - ttfb );
 			} );
 		}
 	}