fix: status of retried test should not change root status

gemini-testing · Mar 21, 2018 · 297c717 · 297c717
1 parent 57013d5
commit 297c717
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 30 deletions.
diff --git a/lib/report-builder-factory/report-builder.js b/lib/report-builder-factory/report-builder.js
@@ -6,7 +6,7 @@ const _ = require('lodash');
 const fs = require('fs-extra');
 const {IDLE, RUNNING, SUCCESS, FAIL, ERROR, SKIPPED, UPDATED} = require('../constants/test-statuses');
 const {getCurrentPath, getReferencePath, getDiffPath, logger} = require('../server-utils');
-const {isSuccessStatus, isFailStatus, isErroredStatus, isIdleStatus} = require('../common-utils');
+const {setStatusForBranch} = require('../static/modules/utils');
 
 const NO_STATE = 'NO_STATE';
 
@@ -111,29 +111,29 @@ module.exports = class ReportBuilder {
         return this;
     }
 
-    _createTestResult(result, status) {
+    _createTestResult(result, props) {
         const {browserId, suite, sessionId, description} = result;
         const {baseHost} = this._pluginConfig;
         const suiteUrl = suite.getUrl({browserId, baseHost});
         const metaInfo = {url: suite.fullUrl, file: suite.file, sessionId};
 
-        return Object.assign({suiteUrl, name: browserId, metaInfo, status, description});
+        return Object.assign({suiteUrl, name: browserId, metaInfo, description}, props);
     }
 
     _addTestResult(result, props) {
         result = this.format(result);
-        const testResult = _.assign(this._createTestResult(result), props);
+        const testResult = this._createTestResult(result, props);
         const {browserId, suite} = result;
         const {status} = props;
 
         const suitePath = suite.path.concat(result.state ? result.state.name : NO_STATE);
         const node = findOrCreate(this._tree, suitePath, status);
         node.browsers = Array.isArray(node.browsers) ? node.browsers : [];
-
         const existing = _.findIndex(node.browsers, {name: browserId});
 
         if (existing === -1) {
             node.browsers.push({name: browserId, result: testResult, retries: []});
+            setStatusForBranch(this._tree, node.suitePath, status);
 
             return;
         }
@@ -148,6 +148,7 @@ module.exports = class ReportBuilder {
         }
 
         stateInBrowser.result = testResult;
+        setStatusForBranch(this._tree, node.suitePath, status);
     }
 
     save() {
@@ -199,22 +200,7 @@ module.exports = class ReportBuilder {
     }
 };
 
-function shouldUpdateStatus(prevStatus, status) {
-    const isFailedStatus = (status) => isFailStatus(status) || isErroredStatus(status);
-    const isForcedStatus = isFailedStatus(status) || isIdleStatus(status);
-
-    if (!prevStatus || isForcedStatus || (!isFailedStatus(prevStatus) && isSuccessStatus(status))) {
-        return true;
-    }
-
-    return false;
-}
-
-function findOrCreate(node, statePath, status) {
-    if (shouldUpdateStatus(node.status, status)) {
-        node.status = status;
-    }
-
+function findOrCreate(node, statePath) {
     if (statePath.length === 0) {
         return node;
     }
@@ -238,5 +224,5 @@ function findOrCreate(node, statePath, status) {
         node.children.push(child);
     }
 
-    return findOrCreate(child, statePath, status);
+    return findOrCreate(child, statePath);
 }
diff --git a/lib/static/modules/utils.js b/lib/static/modules/utils.js
@@ -79,15 +79,17 @@ function findNode(node, suitePath) {
 
 function setStatusForBranch(nodes, suitePath, status) {
     const node = findNode(nodes, suitePath);
-    if (node) {
-        if ((isSuccessStatus(status) || isUpdatedStatus(status)) && hasFails(node)) {
-            return;
-        }
-
-        node.status = status;
-        suitePath = suitePath.slice(0, -1);
-        setStatusForBranch(nodes, suitePath, status);
+    if (!node) {
+        return;
     }
+
+    if ((isSuccessStatus(status) || isUpdatedStatus(status)) && hasFails(node)) {
+        return;
+    }
+
+    node.status = status;
+    suitePath = suitePath.slice(0, -1);
+    setStatusForBranch(nodes, suitePath, status);
 }
 
 module.exports = {

diff --git a/test/lib/report-builder-factory/report-builder.js b/test/lib/report-builder-factory/report-builder.js
@@ -209,6 +209,22 @@ describe('ReportBuilder', () => {
             const suiteResult = getSuiteResult_(reportBuilder);
             assert.equal(suiteResult.status, FAIL);
         });
+
+        [
+            {status: 'failed', hasDiff: true},
+            {status: 'errored', hasDiff: false}
+        ].forEach(({status, hasDiff}) => {
+            it(`should rewrite suite status to "success" if it ${status} on first attempt`, () => {
+                const reportBuilder = mkReportBuilder_();
+                const test = stubTest_({browserId: 'bro', hasDiff: () => hasDiff});
+
+                reportBuilder.addRetry(test);
+                reportBuilder.addSuccess(test);
+
+                const suiteResult = getSuiteResult_(reportBuilder);
+                assert.equal(suiteResult.status, SUCCESS);
+            });
+        });
     });
 
     describe('addRetry', () => {