Skip to content

Commit

Permalink
Fix incorrect results in the flaky tests reporter (#40567)
Browse files Browse the repository at this point in the history
* Fix duplicate function name

* Allow more than one line breaks

* Gracefully handle errors when test results is missing

* Refactor and fix dumb mistake again
  • Loading branch information
kevin940726 authored Apr 26, 2022
1 parent f705d1c commit 574bb66
Showing 1 changed file with 56 additions and 64 deletions.
120 changes: 56 additions & 64 deletions .github/report-flaky-tests/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ const metaData = {
let issue;

if ( reportedIssue ) {
let body = reportedIssue.body;
const body = reportedIssue.body;
const meta = metaData.get( body );

if ( isTrunk ) {
Expand All @@ -88,35 +88,39 @@ const metaData = {
}
}

let testResultsList = body
.slice(
body.indexOf( TEST_RESULTS_LIST.open ) +
TEST_RESULTS_LIST.open.length,
body.indexOf( TEST_RESULTS_LIST.close )
)
/**
* Split the text from:
* ```
* <!-- __TEST_RESULT__ --> Test result 1 <!-- /__TEST_RESULT__ -->
* ...
* <!-- __TEST_RESULT__ --> Test result 2 <!-- /__TEST_RESULT__ -->
* <!-- __TEST_RESULT__ --> Test result 3 <!-- /__TEST_RESULT__ -->
* ```
*
* into:
* ```
* [
* '<!-- __TEST_RESULT__ --> Test result 1 <!-- /__TEST_RESULT__ -->',
* '<!-- __TEST_RESULT__ --> Test result 2 <!-- /__TEST_RESULT__ -->',
* '<!-- __TEST_RESULT__ --> Test result 3 <!-- /__TEST_RESULT__ -->',
* ]
* ```
*/
.split(
new RegExp(
`(?<=${ TEST_RESULT.close })\n(?:\.\.\.\n)?(?=${ TEST_RESULT.open })`
)
);
let testResultsList =
body.includes( TEST_RESULTS_LIST.open ) &&
body.includes( TEST_RESULTS_LIST.close )
? body
.slice(
body.indexOf( TEST_RESULTS_LIST.open ) +
TEST_RESULTS_LIST.open.length,
body.indexOf( TEST_RESULTS_LIST.close )
)
/**
* Split the text from:
* ```
* <!-- __TEST_RESULT__ --> Test result 1 <!-- /__TEST_RESULT__ -->
* ...
* <!-- __TEST_RESULT__ --> Test result 2 <!-- /__TEST_RESULT__ -->
* <!-- __TEST_RESULT__ --> Test result 3 <!-- /__TEST_RESULT__ -->
* ```
*
* into:
* ```
* [
* '<!-- __TEST_RESULT__ --> Test result 1 <!-- /__TEST_RESULT__ -->',
* '<!-- __TEST_RESULT__ --> Test result 2 <!-- /__TEST_RESULT__ -->',
* '<!-- __TEST_RESULT__ --> Test result 3 <!-- /__TEST_RESULT__ -->',
* ]
* ```
*/
.split(
new RegExp(
`(?<=${ TEST_RESULT.close })\n+(?:\.\.\.\n+)?(?=${ TEST_RESULT.open })`
)
)
: [];
// GitHub issues has character limits on issue's body,
// so we only preserve the first and the 9 latest results.
if ( testResultsList.length > 10 ) {
Expand All @@ -127,25 +131,26 @@ const metaData = {
];
}

// Reconstruct the body with the description + previous errors + new error.
body =
renderIssueDescription( { meta, testTitle, testPath } ) +
[
TEST_RESULTS_LIST.open,
...testResultsList,
renderTestErrorMessage( {
testRunner,
testPath,
testResults,
} ),
TEST_RESULTS_LIST.close,
].join( '\n' );
// Concat the test results list with the latest test results.
const formattedTestResults = [
...testResultsList,
formatTestResults( {
testRunner,
testPath,
testResults,
} ),
].join( '\n' );

const response = await octokit.rest.issues.update( {
...github.context.repo,
issue_number: reportedIssue.number,
state: 'open',
body,
body: renderIssueBody( {
meta,
testTitle,
testPath,
formattedTestResults,
} ),
} );

issue = response.data;
Expand All @@ -166,10 +171,9 @@ const metaData = {
title: issueTitle,
body: renderIssueBody( {
meta,
testRunner,
testTitle,
testPath,
testResults,
formattedTestResults: formatTestResults( testResults ),
} ),
labels: [ label ],
} );
Expand Down Expand Up @@ -235,18 +239,10 @@ function getIssueTitle( testTitle ) {

function renderIssueBody( {
meta,
testRunner,
testTitle,
testPath,
testResults,
formattedTestResults,
} ) {
return (
renderIssueDescription( { meta, testTitle, testPath } ) +
renderTestResults( { testRunner, testPath, testResults } )
);
}

function renderIssueDescription( { meta, testTitle, testPath } ) {
return `${ metaData.render( meta ) }
**Flaky test detected. This is an auto-generated issue by GitHub Actions. Please do NOT edit this manually.**
Expand All @@ -260,17 +256,13 @@ ${ testTitle }
\`${ meta.failedTimes } / ${ meta.totalCommits + meta.failedTimes }\` runs
## Errors
`;
}

function renderTestResults( { testRunner, testPath, testResults } ) {
return `${ TEST_RESULTS_LIST.open }
${ renderTestErrorMessage( { testRunner, testPath, testResults } ) }
${ TEST_RESULTS_LIST.open }
${ formattedTestResults }
${ TEST_RESULTS_LIST.close }
`;
}

function renderTestResults( { testRunner, testResults, testPath } ) {
function formatTestErrorMessage( { testRunner, testResults, testPath } ) {
switch ( testRunner ) {
case '@playwright/test': {
// Could do a slightly better formatting than this.
Expand All @@ -297,7 +289,7 @@ function renderTestResults( { testRunner, testResults, testPath } ) {
}
}

function renderTestErrorMessage( { testRunner, testPath, testResults } ) {
function formatTestResults( { testRunner, testPath, testResults } ) {
const date = new Date().toISOString();
// It will look something like this without formatting:
// ▶ [2021-08-31T16:15:19.875Z] Test passed after 2 failed attempts on trunk
Expand All @@ -310,7 +302,7 @@ function renderTestErrorMessage( { testRunner, testPath, testResults } ) {
</summary>
\`\`\`
${ renderTestResults( { testRunner, testPath, testResults } ) }
${ formatTestErrorMessage( { testRunner, testPath, testResults } ) }
\`\`\`
</details>${ TEST_RESULT.close }`;
}
Expand Down

0 comments on commit 574bb66

Please sign in to comment.