fix: perf issue when many errors on cucumber

- use memoization to avoid use globbing multiple time for the same path
deblockt · Nov 4, 2021 · a549247 · a549247
1 parent 0d99491
commit a549247
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 25 deletions.
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -25,6 +25,15 @@ jobs:
           access-token: ${{ secrets.GITHUB_TOKEN }}
           path: "**/cucumber-report.json"
           show-number-of-error-on-check-title: false
+      - uses: ./
+        name: "fail because of error with all annotation"
+        with:
+          name: "failed with all annotation"
+          access-token: ${{ secrets.GITHUB_TOKEN }}
+          path: "**/cucumber-report.json"
+          show-number-of-error-on-check-title: false
+          annotation-status-on-undefined: notice
+          annotation-status-on-pending: notice
       - uses: ./
         name: "fail because of undefined test"
         with:
@@ -40,7 +49,7 @@ jobs:
           access-token: ${{ secrets.GITHUB_TOKEN }}
           path: "**/cucumber-report-non-failed.json"
           check-status-on-pending: "failure"
-          annotation-status-on-pending: "failure"     
+          annotation-status-on-pending: "failure"
       - uses: ./
         name: "neutral because of error"
         with:

diff --git a/index.js b/index.js
@@ -4,29 +4,42 @@ const glob = require("@actions/glob");
 const fs = require("fs");
 const reportReader = require('./reportReader');
 
+function memoize(fn) {
+    const cache = {};
+
+    return (...args) => {
+        let argsString = JSON.stringify(args);
+        return argsString in cache
+          ? cache[argsString]
+          : (cache[argsString] = fn(...args));
+    }
+}
+
 async function findBestFileMatch(file) {
-    if (file.startsWith('classpath:')) {
-        file = file.substring(10);
+    let searchFile = file;
+    if (searchFile.startsWith('classpath:')) {
+        searchFile = searchFile.substring(10);
     }
-    const glober = await glob.create('**/' + file, {
+    const globber = await glob.create('**/' + searchFile, {
         followSymbolicLinks: false,
     });
-
-    const match = [];
-    for await (const featureFile of glober.globGenerator()) {
+    const files = await globber.glob()
+    if (files.length > 0) {
+        const featureFile = files[0];
         const repoName = github.context.repo.repo;
         const indexOfRepoName = featureFile.indexOf(repoName);
-        // convert /home/...../repoName/repoName/filePath to filePath
         const filePathWithoutWorkspace = featureFile.substring(indexOfRepoName + repoName.length * 2 + 2);
-        match.push(filePathWithoutWorkspace);
+        return filePathWithoutWorkspace;
     }
 
-    return match[0];
+    return undefined;
 }
 
+const memoizedFindBestFileMatch = memoize(findBestFileMatch)
+
 async function buildStepAnnotation(cucumberError, status, errorType) {
     return {
-        path: (await findBestFileMatch(cucumberError.file)) || cucumberError.file,
+        path: (await memoizedFindBestFileMatch(cucumberError.file)) || cucumberError.file,
         start_line: cucumberError.line,
         end_line: cucumberError.line,
         start_column: 0,
@@ -38,15 +51,15 @@ async function buildStepAnnotation(cucumberError, status, errorType) {
 }
 
 async function buildErrorAnnotations(cucumberError, statusOnError) {
-    return buildStepAnnotation(cucumberError, statusOnError, 'Failed');
+    return await buildStepAnnotation(cucumberError, statusOnError, 'Failed');
 }
 
 async function buildUndefinedAnnotation(cucumberError, statusOnSkipped) {
-    return buildStepAnnotation(cucumberError, statusOnSkipped, 'Undefined');
+    return await buildStepAnnotation(cucumberError, statusOnSkipped, 'Undefined');
 }
 
 async function buildPendingAnnotation(cucumberError, statusOnPending) {
-    return buildStepAnnotation(cucumberError, statusOnPending, 'Pending');
+    return await buildStepAnnotation(cucumberError, statusOnPending, 'Pending');
 }
 
 (async() => {
@@ -60,13 +73,13 @@ async function buildPendingAnnotation(cucumberError, statusOnPending) {
     const annotationStatusOnUndefined = core.getInput('annotation-status-on-undefined');
     const annotationStatusOnPending = core.getInput('annotation-status-on-pending');
     const showNumberOfErrorOnCheckTitle = core.getInput('show-number-of-error-on-check-title');
-    
+
     const globber = await glob.create(inputPath, {
         followSymbolicLinks: false,
     });
 
     core.info("start to read cucumber logs using path " + inputPath);
-    
+
     for await (const cucumberReportFile of globber.globGenerator()) {
         core.info("found cucumber report " + cucumberReportFile);
 
@@ -86,13 +99,14 @@ async function buildPendingAnnotation(cucumberError, statusOnPending) {
             'pending': globalInformation.pendingStepNumber,
             'passed': globalInformation.succeedStepsNumber
         };
-        const summary = 
-               buildSummary(globalInformation.scenarioNumber, 'Scenarios', summaryScenario) 
-            + '\n' 
+        const summary =
+               buildSummary(globalInformation.scenarioNumber, 'Scenarios', summaryScenario)
+            + '\n'
             + buildSummary(globalInformation.stepsNumber, 'Steps', summarySteps);
 
         const errors = reportReader.failedSteps(reportResult);
         var errorAnnotations = await Promise.all(errors.map(e => buildErrorAnnotations(e, annotationStatusOnError)));
+
         if (annotationStatusOnUndefined) {
             const undefined = reportReader.undefinedSteps(reportResult);
             var undefinedAnnotations = await Promise.all(undefined.map(e => buildUndefinedAnnotation(e, annotationStatusOnUndefined)));
@@ -119,8 +133,9 @@ async function buildPendingAnnotation(cucumberError, statusOnPending) {
                 title: 'Cucumber report summary',
                 message: summary,
             },
-            ...errorAnnotations 
+            ...errorAnnotations
         ];
+
         var additionnalTitleInfo = '';
         if (showNumberOfErrorOnCheckTitle == 'true' && globalInformation.failedScenarioNumber > 0) {
             additionnalTitleInfo = ` (${globalInformation.failedScenarioNumber} error${globalInformation.failedScenarioNumber > 1 ? 's': ''})`;
@@ -149,8 +164,9 @@ async function buildPendingAnnotation(cucumberError, statusOnPending) {
           };
 
         core.info(summary);
+
         core.info("send global cucumber report data");
-        const octokit = github.getOctokit(accessToken); 
+        const octokit = github.getOctokit(accessToken);
         await octokit.checks.create(createCheckRequest);
     }
 })();

diff --git a/reportReader.js b/reportReader.js
@@ -4,7 +4,7 @@ module.exports.globalInformation = (report) => {
     return report
         .map(fileReport => globalFileInformation(fileReport))
         .reduce((a, b) => sum(a, b));
-} 
+}
 
 module.exports.failedSteps = (report) => {
     return report
@@ -98,10 +98,9 @@ function globalFileInformation(reportFile) {
         skippedStepsNumber: skippedStepsNumber,
         undefinedStepsNumber: undefinedStepsNumber,
         pendingStepNumber: pendingStepNumber,
-        succeedStepsNumber: stepsNumber - failedStepsNumber - skippedStepsNumber - undefinedStepsNumber - pendingStepNumber 
+        succeedStepsNumber: stepsNumber - failedStepsNumber - skippedStepsNumber - undefinedStepsNumber - pendingStepNumber
     };
 
-    console.log(result)
     return result;
 }
 
@@ -155,7 +154,7 @@ function buildStepData(fileReport, scenario, getStepsFunction) {
     const skippedStep = getStepsFunction(scenario)[0];
     return {
         file: fileReport.uri,
-        line: skippedStep.line, 
+        line: skippedStep.line,
         title: scenario.name,
         step: skippedStep.name,
         error: skippedStep.result.error_message