fix(gui): incorrect tests counting (#396)

- incorrect counting of the number of images on click "Accept opened"
- incorrect counting of the number of errors in the "Group by error" mode
- incorrect counting of the number of tests to run by button "Retry failed tests"

lib/static/modules/selectors/tree.js

@@ -3,7 +3,7 @@ import {createSelector} from 'reselect';
 import {getViewMode} from './view';
 import viewModes from '../../../constants/view-modes';
 import {isIdleStatus} from '../../../common-utils';
-import {isNodeFailed, isNodeSuccessful, isAcceptable} from '../utils';
+import {isNodeFailed, isNodeSuccessful, isAcceptable, iterateSuites} from '../utils';
 
 const getSuites = (state) => state.tree.suites.byId;
 const getSuitesStates = (state) => state.tree.suites.stateById;
@@ -100,7 +100,7 @@ export const getAcceptableImagesByStateName = createSelector(
     }
 );
 
-const getActiveYoungestSuites = createSelector(
+const getActiveTestSuites = createSelector(
     getAllRootSuiteIds, getSuites, getSuitesStates,
     (rootSuiteIds, suites, suitesStates) => {
         const filterFn = (suite) => {
@@ -113,14 +113,12 @@ const getActiveYoungestSuites = createSelector(
             .map((rootSuiteId) => suites[rootSuiteId])
             .filter(filterFn);
 
-        return flatMap(activeRootSuites, (rootSuite) => {
-            return getYoungestSuites(rootSuite, {suites}, filterFn);
-        });
+        return flatMap(activeRootSuites, (rootSuite) => getTestSuites(rootSuite, {suites}, filterFn));
     }
 );
 
 const getActiveBrowsers = createSelector(
-    getActiveYoungestSuites, getBrowsers, getBrowsersStates, getActiveYoungestSuites,
+    getActiveTestSuites, getBrowsers, getBrowsersStates, getActiveTestSuites,
     (activeYoungestSuites, browsers, browsersStates) => {
         return flatMap(activeYoungestSuites, (suite) => suite.browserIds)
             .filter((browserId) => {
@@ -172,29 +170,23 @@ export const getVisibleRootSuiteIds = createSelector(
 export function getSuiteResults(node, tree, filterFn = identity) {
     const {suites, browsers, results} = tree;
 
-    if (node.resultIds) {
-        return [].concat(filterFn(node.resultIds)).map((resultId) => results[resultId]);
-    }
-
-    if (node.browserIds) {
-        return flatMap(node.browserIds, (browserId) => {
-            return getSuiteResults(browsers[browserId], tree, filterFn);
-        });
-    }
-
-    return flatMap(node.suiteIds, (suiteId) => getSuiteResults(suites[suiteId], tree, filterFn));
+    return iterateSuites(node, {
+        suiteCb: (suiteId) => getSuiteResults(suites[suiteId], tree, filterFn),
+        browserCb: (browserId) => {
+            return [].concat(filterFn(browsers[browserId].resultIds)).map((resultId) => results[resultId]);
+        }
+    });
 }
 
-function getYoungestSuites(suite, tree, filterFn = identity) {
+function getTestSuites(suite, tree, filterFn = identity) {
     if (!filterFn(suite)) {
-        return;
+        return [];
     }
 
-    if (suite.browserIds) {
-        return suite;
-    }
-
-    return compact(flatMap(suite.suiteIds, (suiteId) => getYoungestSuites(tree.suites[suiteId], tree, filterFn)));
+    return iterateSuites(suite, {
+        suiteCb: (suiteId) => getTestSuites(tree.suites[suiteId], tree, filterFn),
+        browserIdsCb: (_, parent) => [parent]
+    });
 }
 
 function getSuiteBrowsers(suite, tree) {
@@ -222,21 +214,24 @@ function getBrowserImages(node, tree) {
 export function getFailedSuiteResults(tree) {
     const failedRootSuites = tree.suites.failedRootIds.map((suiteId) => tree.suites.byId[suiteId]);
     const failedTestSuites = compact(flatMap(failedRootSuites, (failedRootSuite) => getFailedTestSuites(failedRootSuite, tree.suites.byId)));
-    const preparedTree = {suites: tree.suites.byId, browsers: tree.browsers.byId, results: tree.results.byId};
 
-    return flatMap(failedTestSuites, (suite) => getSuiteResults(suite, preparedTree));
+    return flatMap(failedTestSuites, (suite) => iterateSuites(suite, {
+        browserCb: (browserId) => {
+            const browser = tree.browsers.byId[browserId];
+            return browser.resultIds.map((resultId) => tree.results.byId[resultId]);
+        }
+    }));
 }
 
 function getFailedTestSuites(suite, suites) {
     if (!isNodeFailed(suite)) {
-        return;
+        return [];
     }
 
-    if (suite.browserIds) {
-        return suite;
-    }
-
-    return flatMap(suite.suiteIds, (suiteId) => getFailedTestSuites(suites[suiteId], suites));
+    return iterateSuites(suite, {
+        suiteCb: (suiteId) => getFailedTestSuites(suites[suiteId], suites),
+        browserIdsCb: (_, parent) => [parent]
+    });
 }
 
 function getLastImageByStateName(image, browsers, results, images) {

lib/static/modules/utils.js

@@ -1,7 +1,7 @@
 'use strict';
 
 const url = require('url');
-const {get, isEmpty, find} = require('lodash');
+const {get, isEmpty, find, isFunction, flatMap} = require('lodash');
 const {isIdleStatus, isSuccessStatus, isUpdatedStatus, isFailStatus, isErroredStatus, isSkippedStatus} = require('../../common-utils');
 const {getCommonErrors} = require('../../constants/errors');
 const viewModes = require('../../constants/view-modes');
@@ -118,6 +118,23 @@ function shouldShowBrowser(browser, filteredBrowsers) {
     return browserVersionsToFilterBy.includes(browser.version);
 }
 
+function iterateSuites(node, {suiteCb, browserCb, browserIdsCb}) {
+    let resultFromBrowsers = [];
+    let resultFromSuites = [];
+
+    if (node.browserIds && [browserCb, browserIdsCb].some(isFunction)) {
+        resultFromBrowsers = browserIdsCb
+            ? browserIdsCb(node.browserIds, node)
+            : flatMap(node.browserIds, (browserId) => browserCb(browserId, node));
+    }
+
+    if (node.suiteIds && isFunction(suiteCb)) {
+        resultFromSuites = flatMap(node.suiteIds, (suiteId) => suiteCb(suiteId, node));
+    }
+
+    return [...resultFromBrowsers, ...resultFromSuites];
+}
+
 module.exports = {
     isNoRefImageError,
     isAssertViewError,
@@ -133,5 +150,6 @@ module.exports = {
     getHttpErrorMessage,
     isTestNameMatchFilters,
     isStatusMatchViewMode,
-    shouldShowBrowser
+    shouldShowBrowser,
+    iterateSuites
 };

test/unit/lib/static/modules/selectors/tree.js

@@ -1,9 +1,11 @@
 import {SUCCESS, FAIL, IDLE} from 'lib/constants/test-statuses';
 import {
     getAcceptableImagesByStateName,
+    getAcceptableOpenedImageIds,
     mkGetLastImageByStateName,
     areAllRootSuitesIdle,
-    getFailedTests
+    getFailedTests,
+    getFailedSuiteResults
 } from 'lib/static/modules/selectors/tree';
 import {mkSuite, mkBrowser, mkResult, mkImage, mkStateTree, mkStateView} from '../../state-utils';
 
@@ -120,6 +122,125 @@ describe('tree selectors', () => {
         });
     });
 
+    describe('"getAcceptableOpenedImageIds" selector', () => {
+        describe('should not return images if', () => {
+            const _mkStateTree = () => {
+                const suitesById = {...mkSuite({id: 's1', browserIds: ['b1']})};
+                const suitesStateById = {s1: {shouldBeShown: true, shouldBeOpened: true}};
+                const suitesAllRootIds = ['s1'];
+
+                const browsersById = {...mkBrowser({id: 'b1', resultIds: ['r1']})};
+                const browsersStateById = {b1: {shouldBeShown: true, shouldBeOpened: true, retryIndex: 0}};
+
+                const resultsById = {...mkResult({id: 'r1', parentId: 'b1', imageIds: ['img1']})};
+
+                const imagesById = {...mkImage({id: 'img1', parentId: 'r1', stateName: 'first', status: FAIL})};
+                const imagesStateById = {img1: {shouldBeOpened: true}};
+
+                return mkStateTree({
+                    suitesById, suitesStateById, suitesAllRootIds, browsersById, browsersStateById,
+                    resultsById, imagesById, imagesStateById
+                });
+            };
+
+            it('suite is not shown', () => {
+                const tree = _mkStateTree();
+                tree.suites.stateById['s1'].shouldBeShown = false;
+
+                const state = mkState({tree});
+
+                assert.deepEqual(getAcceptableOpenedImageIds(state), []);
+            });
+
+            it('suite is not opened', () => {
+                const tree = _mkStateTree();
+                tree.suites.stateById['s1'].shouldBeOpened = false;
+
+                const state = mkState({tree});
+
+                assert.deepEqual(getAcceptableOpenedImageIds(state), []);
+            });
+
+            it('browser is not shown', () => {
+                const tree = _mkStateTree();
+                tree.browsers.stateById['b1'].shouldBeShown = false;
+
+                const state = mkState({tree});
+
+                assert.deepEqual(getAcceptableOpenedImageIds(state), []);
+            });
+
+            it('browser is not opened', () => {
+                const tree = _mkStateTree();
+                tree.browsers.stateById['b1'].shouldBeOpened = false;
+
+                const state = mkState({tree});
+
+                assert.deepEqual(getAcceptableOpenedImageIds(state), []);
+            });
+
+            it('image is not opened', () => {
+                const tree = _mkStateTree();
+                tree.images.stateById['img1'].shouldBeOpened = false;
+
+                const state = mkState({tree});
+
+                assert.deepEqual(getAcceptableOpenedImageIds(state), []);
+            });
+
+            it('image is not acceptable', () => {
+                const tree = _mkStateTree();
+                tree.images.byId['img1'].status = SUCCESS;
+
+                const state = mkState({tree});
+
+                assert.deepEqual(getAcceptableOpenedImageIds(state), []);
+            });
+        });
+
+        it('should return opened images', () => {
+            const suitesById = {
+                ...mkSuite({id: 's1', suiteIds: ['s2'], browserIds: ['b1']}),
+                ...mkSuite({id: 's2', browserIds: ['b2']})
+            };
+            const suitesStateById = {
+                s1: {shouldBeShown: true, shouldBeOpened: true},
+                s2: {shouldBeShown: true, shouldBeOpened: true}
+            };
+            const suitesAllRootIds = ['s1'];
+
+            const browsersById = {
+                ...mkBrowser({id: 'b1', resultIds: ['r1']}),
+                ...mkBrowser({id: 'b2', resultIds: ['r2']})
+            };
+            const browsersStateById = {
+                b1: {shouldBeShown: true, shouldBeOpened: true, retryIndex: 0},
+                b2: {shouldBeShown: true, shouldBeOpened: true, retryIndex: 0}
+            };
+
+            const resultsById = {
+                ...mkResult({id: 'r1', parentId: 'b1', imageIds: ['img1']}),
+                ...mkResult({id: 'r2', parentId: 'b2', imageIds: ['img2']})
+            };
+            const imagesById = {
+                ...mkImage({id: 'img1', parentId: 'r1', stateName: 'first', status: FAIL}),
+                ...mkImage({id: 'img2', parentId: 'r2', stateName: 'second', status: FAIL})
+            };
+            const imagesStateById = {img1: {shouldBeOpened: true}, img2: {shouldBeOpened: true}};
+
+            const tree = mkStateTree({
+                suitesById, suitesStateById, suitesAllRootIds, browsersById, browsersStateById,
+                resultsById, imagesById, imagesStateById
+            });
+            const state = mkState({tree});
+
+            assert.deepEqual(
+                getAcceptableOpenedImageIds(state),
+                ['img1', 'img2']
+            );
+        });
+    });
+
     describe('"mkGetLastImageByStateName" factory', () => {
         it('should return image with passed "id" if there are no images with the same state name', () => {
             const browsersById = mkBrowser({id: 'b', resultIds: ['r1', 'r2']});
@@ -185,25 +306,75 @@ describe('tree selectors', () => {
 
     describe('"getFailedTests" selector', () => {
         it('should return failed tests with metaInfo', () => {
-            const suitesById = mkSuite({id: 's1', status: FAIL, browserIds: ['b']});
-            const browsersById = mkBrowser({id: 'b', name: 'browser', parentId: 'test', resultIds: ['r1']});
-            const resultsById = mkResult({id: 'r1', parentId: 'b', status: FAIL, metaInfo: {param: 'value'}});
+            const suitesById = {
+                ...mkSuite({id: 's1', suiteIds: ['s2'], browserIds: ['b1'], status: FAIL}),
+                ...mkSuite({id: 's2', browserIds: ['b2'], status: FAIL})
+            };
+            const suitesFailedRootIds = ['s1'];
+            const browsersById = {
+                ...mkBrowser({id: 'b1', name: 'browser1', parentId: 's1', resultIds: ['r1']}),
+                ...mkBrowser({id: 'b2', name: 'browser2', parentId: 's2', resultIds: ['r2']})
+            };
+            const resultsById = {
+                ...mkResult({id: 'r1', parentId: 'b1', status: FAIL, metaInfo: {param: 'value1'}}),
+                ...mkResult({id: 'r2', parentId: 'b2', status: FAIL, metaInfo: {param: 'value2'}})
+            };
 
             const tree = mkStateTree({
-                suitesFailedRootIds: ['s1'],
                 suitesById,
+                suitesFailedRootIds,
                 browsersById,
                 resultsById
             });
             const state = mkState({tree});
 
             assert.deepEqual(
                 getFailedTests(state),
-                [{
-                    'browserName': 'browser',
-                    'metaInfo': {param: 'value'},
-                    'testName': 'test'
-                }]
+                [
+                    {
+                        'browserName': 'browser1',
+                        'metaInfo': {param: 'value1'},
+                        'testName': 's1'
+                    },
+                    {
+                        'browserName': 'browser2',
+                        'metaInfo': {param: 'value2'},
+                        'testName': 's2'
+                    }
+                ]
+            );
+        });
+    });
+
+    describe('"getFailedSuiteResults" helper', () => {
+        it('should return all failed results', () => {
+            const suitesById = {
+                ...mkSuite({id: 's1', suiteIds: ['s2'], browserIds: ['b1'], status: FAIL}),
+                ...mkSuite({id: 's2', browserIds: ['b2'], status: FAIL}),
+                ...mkSuite({id: 's3', browserIds: ['b3'], status: SUCCESS})
+            };
+            const suitesFailedRootIds = ['s1'];
+            const browsersById = {
+                ...mkBrowser({id: 'b1', resultIds: ['r1']}),
+                ...mkBrowser({id: 'b2', resultIds: ['r2']}),
+                ...mkBrowser({id: 'b3', resultIds: ['r3']})
+            };
+            const resultsById = {
+                ...mkResult({id: 'r1', parentId: 'b1'}),
+                ...mkResult({id: 'r2', parentId: 'b2'}),
+                ...mkResult({id: 'r3', parentId: 'b3'})
+            };
+
+            const tree = mkStateTree({
+                suitesById,
+                suitesFailedRootIds,
+                browsersById,
+                resultsById
+            });
+
+            assert.deepEqual(
+                getFailedSuiteResults(tree),
+                [resultsById.r1, resultsById.r2]
             );
         });
     });