fix: skipped priority is less than red & green states

lib/common-utils.js

@@ -1,10 +1,26 @@
 'use strict';
 
-const {SUCCESS, FAIL, ERROR, SKIPPED, UPDATED, IDLE} = require('./constants/test-statuses');
+const {SUCCESS, FAIL, ERROR, SKIPPED, UPDATED, IDLE, RUNNING, QUEUED} = require('./constants/test-statuses');
+const statusPriority = [
+    // non-final
+    RUNNING, QUEUED, IDLE,
+
+    // final
+    ERROR, FAIL, UPDATED, SUCCESS, SKIPPED
+];
 
 exports.isSuccessStatus = (status) => status === SUCCESS;
 exports.isFailStatus = (status) => status === FAIL;
 exports.isIdleStatus = (status) => status === IDLE;
 exports.isErroredStatus = (status) => status === ERROR;
 exports.isSkippedStatus = (status) => status === SKIPPED;
 exports.isUpdatedStatus = (status) => status === UPDATED;
+
+exports.determineStatus = (statuses) => {
+    const set = new Set(statuses);
+    for (const status of statusPriority) {
+        if (set.has(status)) {
+            return status;
+        }
+    }
+};

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 {logger, getImagesFor, hasImage, prepareCommonJSData} = require('../server-utils');
-const {setStatusForBranch, hasFails, hasNoRefImageErrors} = require('../static/modules/utils');
+const {setStatusForBranch, hasNoRefImageErrors} = require('../static/modules/utils');
 
 const NO_STATE = 'NO_STATE';
 
@@ -140,7 +140,7 @@ module.exports = class ReportBuilder {
             }
 
             node.browsers.push({name: browserId, result: testResult, retries: []});
-            setStatusForBranch(this._tree, node.suitePath, testResult.status);
+            setStatusForBranch(this._tree, node.suitePath);
 
             return formattedResult;
         }
@@ -166,15 +166,13 @@ module.exports = class ReportBuilder {
         const {imagesInfo, status: currentStatus} = stateInBrowser.result;
         stateInBrowser.result = extendTestWithImagePaths(testResult, formattedResult, imagesInfo);
 
-        if (!hasFails(stateInBrowser)) {
-            stateInBrowser.result.status = SUCCESS;
-        } else if (hasNoRefImageErrors(stateInBrowser.result)) {
+        if (hasNoRefImageErrors(stateInBrowser.result)) {
             stateInBrowser.result.status = FAIL;
         } else if (stateInBrowser.result.status === UPDATED) {
             stateInBrowser.result.status = currentStatus;
         }
 
-        setStatusForBranch(this._tree, node.suitePath, testResult.status);
+        setStatusForBranch(this._tree, node.suitePath);
 
         return formattedResult;
     }

lib/static/modules/reducer.js

@@ -147,7 +147,7 @@ function addTestResult(state, action) {
                 Object.assign(b, browserResult);
             }
         });
-        setStatusForBranch(suites, suitePath, browserResult.result.status);
+        setStatusForBranch(suites, suitePath);
         forceUpdateSuiteData(suites, test);
     });
 

lib/static/modules/utils.js

@@ -1,7 +1,7 @@
 'use strict';
 
 const {isArray, find, get, values} = require('lodash');
-const {isSuccessStatus, isFailStatus, isErroredStatus, isSkippedStatus, isUpdatedStatus} = require('../../common-utils');
+const {isFailStatus, isErroredStatus, isSkippedStatus, determineStatus} = require('../../common-utils');
 const {getCommonErrors} = require('../../constants/errors');
 
 const {NO_REF_IMAGE_ERROR} = getCommonErrors();
@@ -91,19 +91,25 @@ function findNode(node, suitePath) {
     return findNode(child, suitePath);
 }
 
-function setStatusForBranch(nodes, suitePath, status) {
+function setStatusForBranch(nodes, suitePath) {
     const node = findNode(nodes, suitePath);
     if (!node) {
         return;
     }
 
-    if ((isSuccessStatus(status) || isUpdatedStatus(status)) && hasFails(node)) {
+    const statuses = node.browsers
+        ? node.browsers.map(({result: {status}}) => status)
+        : node.children.map(({status}) => status);
+
+    const status = determineStatus(statuses);
+
+    // if newly determined status is the same as current status, do nothing
+    if (node.status === status) {
         return;
     }
 
     node.status = status;
-    suitePath = suitePath.slice(0, -1);
-    setStatusForBranch(nodes, suitePath, status);
+    setStatusForBranch(nodes, suitePath.slice(0, -1));
 }
 
 function shouldSuiteBeShown(suite, filterByName) {

test/lib/report-builder-factory/report-builder.js

@@ -230,61 +230,200 @@ describe('ReportBuilder', () => {
             assert.equal(suiteResult.status, SUCCESS);
         });
 
-        it('should rewrite suite status if new status has "failed" type', () => {
-            const reportBuilder = mkReportBuilder_();
+        describe('for one browser', () => {
+            describe('should rewrite status to "skipped" if', () => {
+                it('first attempt was "idle"', () => {
+                    const reportBuilder = mkReportBuilder_();
+                    const test = stubTest_({browserId: 'bro'});
 
-            reportBuilder.addSuccess(stubTest_({browserId: 'bro'}));
-            reportBuilder.addError(stubTest_({browserId: 'another-bro'}));
+                    reportBuilder.addIdle(test);
+                    reportBuilder.addSkipped(test);
 
-            const suiteResult = getSuiteResult_(reportBuilder);
-            assert.equal(suiteResult.status, ERROR);
-        });
+                    const suiteResult = getSuiteResult_(reportBuilder);
+                    assert.equal(suiteResult.status, SKIPPED);
+                });
 
-        it('should rewrite suite status if new status has "forced" type', () => {
-            const reportBuilder = mkReportBuilder_();
+                it('first attempt was "error"', () => {
+                    const reportBuilder = mkReportBuilder_();
+                    const test = stubTest_({browserId: 'bro', hasDiff: () => false});
 
-            reportBuilder.addFail(stubTest_({browserId: 'bro'}));
-            reportBuilder.addIdle(stubTest_({browserId: 'another-bro'}));
+                    reportBuilder.addRetry(test);
+                    reportBuilder.addSkipped(test);
 
-            const suiteResult = getSuiteResult_(reportBuilder);
-            assert.equal(suiteResult.status, IDLE);
-        });
+                    const suiteResult = getSuiteResult_(reportBuilder);
+                    assert.equal(suiteResult.status, SKIPPED);
+                });
 
-        it('should rewrite suite status if new status has "final" result', () => {
-            const reportBuilder = mkReportBuilder_();
+                it('first attempt was "fail"', () => {
+                    const reportBuilder = mkReportBuilder_();
+                    const test = stubTest_({browserId: 'bro', hasDiff: () => true});
 
-            reportBuilder.addIdle(stubTest_({browserId: 'another-bro'}));
-            reportBuilder.addSuccess(stubTest_({browserId: 'bro'}));
+                    reportBuilder.addRetry(test);
+                    reportBuilder.addSkipped(test);
 
-            const suiteResult = getSuiteResult_(reportBuilder);
-            assert.equal(suiteResult.status, SUCCESS);
-        });
+                    const suiteResult = getSuiteResult_(reportBuilder);
+                    assert.equal(suiteResult.status, SKIPPED);
+                });
 
-        it('should not rewrite suite status to "success" if it has failed state', () => {
-            const reportBuilder = mkReportBuilder_();
+                it('first attempt was "updated"', () => {
+                    const reportBuilder = mkReportBuilder_();
+                    const test = stubTest_({browserId: 'bro', hasDiff: () => true});
 
-            reportBuilder.addFail(stubTest_({browserId: 'bro'}));
-            reportBuilder.addSuccess(stubTest_({browserId: 'another-bro'}));
+                    reportBuilder.addUpdated(test);
+                    reportBuilder.addSkipped(test);
 
-            const suiteResult = getSuiteResult_(reportBuilder);
-            assert.equal(suiteResult.status, FAIL);
+                    const suiteResult = getSuiteResult_(reportBuilder);
+                    assert.equal(suiteResult.status, SKIPPED);
+                });
+
+                it('first attempt was "success"', () => {
+                    const reportBuilder = mkReportBuilder_();
+                    const test = stubTest_({browserId: 'bro', hasDiff: () => true});
+
+                    reportBuilder.addSuccess(test);
+                    reportBuilder.addSkipped(test);
+
+                    const suiteResult = getSuiteResult_(reportBuilder);
+                    assert.equal(suiteResult.status, SKIPPED);
+                });
+            });
+
+            describe('should rewrite status to "success" if', () => {
+                it('first attempt was "idle"', () => {
+                    const reportBuilder = mkReportBuilder_();
+                    const test = stubTest_({browserId: 'bro'});
+
+                    reportBuilder.addIdle(test);
+                    reportBuilder.addSuccess(test);
+
+                    const suiteResult = getSuiteResult_(reportBuilder);
+                    assert.equal(suiteResult.status, SUCCESS);
+                });
+
+                it('first attempt was "error"', () => {
+                    const reportBuilder = mkReportBuilder_();
+                    const test = stubTest_({browserId: 'bro', hasDiff: () => false});
+
+                    reportBuilder.addRetry(test);
+                    reportBuilder.addSuccess(test);
+
+                    const suiteResult = getSuiteResult_(reportBuilder);
+                    assert.equal(suiteResult.status, SUCCESS);
+                });
+
+                it('first attempt was "fail"', () => {
+                    const reportBuilder = mkReportBuilder_();
+                    const test = stubTest_({browserId: 'bro', hasDiff: () => true});
+
+                    reportBuilder.addRetry(test);
+                    reportBuilder.addSuccess(test);
+
+                    const suiteResult = getSuiteResult_(reportBuilder);
+                    assert.equal(suiteResult.status, SUCCESS);
+                });
+            });
         });
 
-        [
-            {status: 'failed', hasDiff: true},
-            {status: 'errored', hasDiff: false}
-        ].forEach(({status, hasDiff}) => {
-            it(`should rewrite suite status to "success" if it ${status} on first attempt`, () => {
+        describe('for several browsers', () => {
+            it('should rewrite suite status if new status has "forced" type', () => {
+                const reportBuilder = mkReportBuilder_();
+
+                reportBuilder.addFail(stubTest_({browserId: 'bro'}));
+                reportBuilder.addIdle(stubTest_({browserId: 'another-bro'}));
+
+                const suiteResult = getSuiteResult_(reportBuilder);
+                assert.equal(suiteResult.status, IDLE);
+            });
+
+            it('should determine "error" if first test has "error"', () => {
+                const reportBuilder = mkReportBuilder_();
+
+                reportBuilder.addError(stubTest_({browserId: 'bro1'}));
+                reportBuilder.addFail(stubTest_({browserId: 'bro2'}));
+                reportBuilder.addUpdated(stubTest_({browserId: 'bro3'}));
+                reportBuilder.addSuccess(stubTest_({browserId: 'bro4'}));
+                reportBuilder.addSkipped(stubTest_({browserId: 'bro5'}));
+
+                const suiteResult = getSuiteResult_(reportBuilder);
+                assert.equal(suiteResult.status, ERROR);
+            });
+
+            it('should determine "error" if last test has "error"', () => {
                 const reportBuilder = mkReportBuilder_();
-                const test1 = stubTest_({browserId: 'bro', hasDiff: () => hasDiff});
-                const test2 = stubTest_({browserId: 'bro', hasDiff: () => hasDiff});
 
-                reportBuilder.addRetry(test1);
-                reportBuilder.addSuccess(test2);
+                reportBuilder.addSkipped(stubTest_({browserId: 'bro5'}));
+                reportBuilder.addSuccess(stubTest_({browserId: 'bro4'}));
+                reportBuilder.addUpdated(stubTest_({browserId: 'bro3'}));
+                reportBuilder.addFail(stubTest_({browserId: 'bro2'}));
+                reportBuilder.addError(stubTest_({browserId: 'bro1'}));
+
+                const suiteResult = getSuiteResult_(reportBuilder);
+                assert.equal(suiteResult.status, ERROR);
+            });
+
+            it('should determine "fail" if first test has "fail"', () => {
+                const reportBuilder = mkReportBuilder_();
+
+                reportBuilder.addFail(stubTest_({browserId: 'bro1'}));
+                reportBuilder.addUpdated(stubTest_({browserId: 'bro2'}));
+                reportBuilder.addSuccess(stubTest_({browserId: 'bro3'}));
+                reportBuilder.addSkipped(stubTest_({browserId: 'bro4'}));
+
+                const suiteResult = getSuiteResult_(reportBuilder);
+                assert.equal(suiteResult.status, FAIL);
+            });
+
+            it('should determine "fail" if last test has "fail"', () => {
+                const reportBuilder = mkReportBuilder_();
+
+                reportBuilder.addSkipped(stubTest_({browserId: 'bro4'}));
+                reportBuilder.addSuccess(stubTest_({browserId: 'bro3'}));
+                reportBuilder.addUpdated(stubTest_({browserId: 'bro2'}));
+                reportBuilder.addFail(stubTest_({browserId: 'bro1'}));
+
+                const suiteResult = getSuiteResult_(reportBuilder);
+                assert.equal(suiteResult.status, FAIL);
+            });
+
+            it('should determine "success" if first test has "success"', () => {
+                const reportBuilder = mkReportBuilder_();
+
+                reportBuilder.addSuccess(stubTest_({browserId: 'bro1'}));
+                reportBuilder.addSkipped(stubTest_({browserId: 'bro2'}));
 
                 const suiteResult = getSuiteResult_(reportBuilder);
                 assert.equal(suiteResult.status, SUCCESS);
             });
+
+            it('should determine "success" if last test has "success"', () => {
+                const reportBuilder = mkReportBuilder_();
+
+                reportBuilder.addSkipped(stubTest_({browserId: 'bro2'}));
+                reportBuilder.addSuccess(stubTest_({browserId: 'bro1'}));
+
+                const suiteResult = getSuiteResult_(reportBuilder);
+                assert.equal(suiteResult.status, SUCCESS);
+            });
+        });
+
+        describe('for retried browsers', () => {
+            [
+                {status: 'failed', hasDiff: true},
+                {status: 'errored', hasDiff: false}
+            ].forEach(({status, hasDiff}) => {
+                it(`should rewrite suite status to "success" if it has ${status}, then skipped test`, () => {
+                    const reportBuilder = mkReportBuilder_();
+                    const test1 = stubTest_({browserId: 'bro', hasDiff: () => hasDiff});
+                    const test2 = stubTest_({browserId: 'bro', hasDiff: () => hasDiff});
+
+                    reportBuilder.addSuccess(stubTest_({browserId: 'another-bro'}));
+                    reportBuilder.addRetry(test1);
+                    reportBuilder.addSkipped(test2);
+
+                    const suiteResult = getSuiteResult_(reportBuilder);
+                    assert.equal(suiteResult.status, SUCCESS);
+                });
+            });
         });
 
         describe('should rewrite suite status to "fail" if test failed with no reference image error', () => {