From 462ba2b615623212112bbc766e2e2f5ce989e2f4 Mon Sep 17 00:00:00 2001 From: DudaGod Date: Thu, 5 Apr 2018 15:22:17 +0300 Subject: [PATCH] fix(gui): accept and retry one test --- .../gemini/report-subscriber.js | 19 ++-- .../hermione/report-subscriber.js | 16 ++- lib/report-builder-factory/report-builder.js | 97 +++++++++++-------- lib/static/modules/actions.js | 8 +- lib/test-adapter/gemini-test-adapter.js | 5 + lib/test-adapter/hermione-test-adapter.js | 5 + .../report-builder-factory/report-builder.js | 7 +- 7 files changed, 89 insertions(+), 68 deletions(-) diff --git a/lib/gui/tool-runner-factory/gemini/report-subscriber.js b/lib/gui/tool-runner-factory/gemini/report-subscriber.js index fa2dc3040..f84d14deb 100644 --- a/lib/gui/tool-runner-factory/gemini/report-subscriber.js +++ b/lib/gui/tool-runner-factory/gemini/report-subscriber.js @@ -31,41 +31,40 @@ module.exports = (gemini, reportBuilder, client, reportPath) => { }); gemini.on(gemini.events.TEST_RESULT, (data) => { - data.equal + const result = data.equal ? reportBuilder.addSuccess(data) : reportBuilder.addFail(data); const testResult = prepareTestResult(data); - saveTestImages(reportBuilder.format(data), reportPath) + saveTestImages(result, reportPath) .then(() => client.emit(clientEvents.TEST_RESULT, testResult)); }); gemini.on(gemini.events.ERROR, (error) => { - reportBuilder.addError(error); + const result = reportBuilder.addError(error); const testResult = prepareTestResult(error); - saveTestCurrentImage(reportBuilder.format(error), reportPath) + saveTestCurrentImage(result, reportPath) .then(() => client.emit(gemini.events.ERROR, testResult)); }); gemini.on(gemini.events.RETRY, (data) => { - reportBuilder.addRetry(data); - const formattedResult = reportBuilder.format(data); + const result = reportBuilder.addRetry(data); - const actionFn = formattedResult.hasDiff() + const actionFn = result.hasDiff() ? saveTestImages : saveTestCurrentImage; - actionFn(formattedResult, reportPath); + actionFn(result, reportPath); }); gemini.on(gemini.events.UPDATE_RESULT, (data) => { - reportBuilder.addUpdated(data); + const result = reportBuilder.addUpdated(data); const testResult = prepareTestResult(data); - updateReferenceImage(reportBuilder.format(data), reportPath) + updateReferenceImage(result, reportPath) .then(() => client.emit(clientEvents.UPDATE_RESULT, testResult)); }); diff --git a/lib/gui/tool-runner-factory/hermione/report-subscriber.js b/lib/gui/tool-runner-factory/hermione/report-subscriber.js index 1137debeb..02259e614 100644 --- a/lib/gui/tool-runner-factory/hermione/report-subscriber.js +++ b/lib/gui/tool-runner-factory/hermione/report-subscriber.js @@ -47,29 +47,27 @@ module.exports = (hermione, reportBuilder, client, reportPath) => { const saveImageFn = getSaveImageFn(formattedResult); const {assertViewState} = formattedResult; - formattedResult.hasDiff() + const result = formattedResult.hasDiff() ? reportBuilder.addFail(data, {assertViewState}) : reportBuilder.addError(data, {assertViewState}); - saveImageFn(formattedResult, reportPath) + saveImageFn(result, reportPath) .then(() => client.emit(clientEvents.TEST_RESULT, testResult)); }); hermione.on(hermione.events.RETRY, (data) => { - const formattedResult = reportBuilder.format(data); - const saveImageFn = getSaveImageFn(formattedResult); - - reportBuilder.addRetry(data); + const result = reportBuilder.addRetry(data); + const saveImageFn = getSaveImageFn(result); - saveImageFn(formattedResult, reportPath); + saveImageFn(result, reportPath); }); // TODO: subscribe on hermione event when it will be added hermione.on(clientEvents.UPDATE_RESULT, (data) => { - reportBuilder.addUpdated(data); + const result = reportBuilder.addUpdated(data); const testResult = prepareTestResult(data); - updateReferenceImage(reportBuilder.format(data), reportPath) + updateReferenceImage(result, reportPath) .then(() => client.emit(clientEvents.UPDATE_RESULT, testResult)); }); diff --git a/lib/report-builder-factory/report-builder.js b/lib/report-builder-factory/report-builder.js index 17e0a18e6..1eda9f745 100644 --- a/lib/report-builder-factory/report-builder.js +++ b/lib/report-builder-factory/report-builder.js @@ -28,68 +28,56 @@ module.exports = class ReportBuilder { } addIdle(result, expectedPath = '') { - this._addTestResult(result, { + return this._addTestResult(this.format(result), { status: IDLE, expectedPath }); } addSkipped(result) { + const formattedResult = this.format(result); const { suite: { skipComment: comment, fullName: suite }, browserId: browser - } = this.format(result); + } = formattedResult; this._skips.push({suite, browser, comment}); - this._addTestResult(result, { + return this._addTestResult(formattedResult, { status: SKIPPED, reason: comment }); } addSuccess(result) { - this._addSuccessResult(result, SUCCESS); + return this._addSuccessResult(this.format(result), SUCCESS); } addUpdated(result) { - this._addSuccessResult(result, UPDATED); + return this._addSuccessResult(this.format(result), UPDATED); } - _addSuccessResult(result, status) { - const formattedResult = this.format(result); - - this._addTestResult(result, { - status, - expectedPath: getReferencePath(formattedResult) - }); + _addSuccessResult(formattedResult, status) { + return this._addTestResult(formattedResult, {status}); } addFail(result, props) { - this._addFailResult(result, props); + return this._addFailResult(this.format(result), props); } - _addFailResult(result, props = {}) { - const formattedResult = this.format(result); - this._addTestResult(result, _.extend({ - status: FAIL, - actualPath: getCurrentPath(formattedResult), - expectedPath: getReferencePath(formattedResult), - diffPath: getDiffPath(formattedResult) - }, props)); + _addFailResult(formattedResult, props = {}) { + return this._addTestResult(formattedResult, _.extend({status: FAIL}, props)); } addError(result, props) { - this._addErrorResult(result, props); + return this._addErrorResult(this.format(result), props); } - _addErrorResult(result, props = {}) { - const formattedResult = this.format(result); - this._addTestResult(result, _.extend({ - actualPath: formattedResult.state ? getCurrentPath(formattedResult) : '', + _addErrorResult(formattedResult, props = {}) { + return this._addTestResult(formattedResult, _.extend({ status: ERROR, image: !!formattedResult.imagePath || !!formattedResult.currentPath || !!formattedResult.screenshot, reason: formattedResult.error @@ -100,9 +88,9 @@ module.exports = class ReportBuilder { const formattedResult = this.format(result); if (formattedResult.hasDiff()) { - this._addFailResult(result); + return this._addFailResult(formattedResult); } else { - this._addErrorResult(result); + return this._addErrorResult(formattedResult); } } @@ -121,22 +109,22 @@ module.exports = class ReportBuilder { return Object.assign({suiteUrl, name: browserId, metaInfo, description}, props); } - _addTestResult(result, props) { - result = this.format(result); - const testResult = this._createTestResult(result, props); - const {browserId, suite} = result; - const {status} = props; + _addTestResult(formattedResult, props) { + const testResult = this._createTestResult(formattedResult, _.extend(props, {attempt: 0})); + const {suite, browserId} = formattedResult; - const suitePath = suite.path.concat(result.state ? result.state.name : NO_STATE); - const node = findOrCreate(this._tree, suitePath, status); + const suitePath = suite.path.concat(formattedResult.state ? formattedResult.state.name : NO_STATE); + const node = findOrCreate(this._tree, suitePath, testResult.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); + formattedResult.attempt = testResult.attempt; + const imagePaths = calcImagePaths(formattedResult, testResult.status); + node.browsers.push({name: browserId, result: _.extend(testResult, imagePaths), retries: []}); + setStatusForBranch(this._tree, node.suitePath, testResult.status); - return; + return formattedResult; } const stateInBrowser = node.browsers[existing]; @@ -144,12 +132,23 @@ module.exports = class ReportBuilder { const statuses = [SKIPPED, RUNNING, IDLE]; - if (!statuses.includes(previousResult.status) && testResult.status !== UPDATED) { - stateInBrowser.retries.push(previousResult); + if (!statuses.includes(previousResult.status)) { + testResult.attempt = testResult.status === UPDATED + ? formattedResult.attempt + : previousResult.attempt + 1; + + if (testResult.status !== UPDATED) { + stateInBrowser.retries.push(previousResult); + } } - stateInBrowser.result = testResult; - setStatusForBranch(this._tree, node.suitePath, status); + formattedResult.attempt = testResult.attempt; + const imagePaths = calcImagePaths(formattedResult, testResult.status); + + stateInBrowser.result = _.extend(testResult, imagePaths); + setStatusForBranch(this._tree, node.suitePath, testResult.status); + + return formattedResult; } save() { @@ -236,3 +235,17 @@ function findOrCreate(node, statePath) { return findOrCreate(child, statePath); } + +function calcImagePaths(formattedResult, status) { + if (status === SUCCESS || status === UPDATED) { + return {expectedPath: getReferencePath(formattedResult)}; + } else if (status === FAIL) { + return { + actualPath: getCurrentPath(formattedResult), + expectedPath: getReferencePath(formattedResult), + diffPath: getDiffPath(formattedResult) + }; + } else if (status === ERROR) { + return {actualPath: formattedResult.state ? getCurrentPath(formattedResult) : ''}; + } +} diff --git a/lib/static/modules/actions.js b/lib/static/modules/actions.js index 9fedd000f..1e19531aa 100644 --- a/lib/static/modules/actions.js +++ b/lib/static/modules/actions.js @@ -67,7 +67,7 @@ export const acceptAll = (fails, actionName = actionNames.ACCEPT_ALL) => { }; export const acceptSuite = (suite, browserId, attempt) => { - return acceptAll(assign({browserId}, suite, {attempt}), actionNames.ACCEPT_SUITE); + return acceptAll(assign({browserId}, suite, {acceptSuiteAttempt: attempt}), actionNames.ACCEPT_SUITE); }; export const suiteBegin = (suite) => ({type: actionNames.SUITE_BEGIN, payload: suite}); @@ -106,10 +106,10 @@ function formatTests(test) { test.browsers = filter(test.browsers, {name: test.browserId}); } - const {suitePath, name, attempt} = test; + const {suitePath, name, acceptSuiteAttempt} = test; return flatMap(test.browsers, (browser) => { - const {metaInfo, assertViewState} = browser.result; + const {metaInfo, assertViewState, attempt} = browser.result; return { suite: {path: suitePath.slice(0, -1)}, @@ -117,7 +117,7 @@ function formatTests(test) { browserId: browser.name, metaInfo, assertViewState, - attempt + attempt: acceptSuiteAttempt >= 0 ? acceptSuiteAttempt : attempt }; }); } diff --git a/lib/test-adapter/gemini-test-adapter.js b/lib/test-adapter/gemini-test-adapter.js index c3f25aae6..20dda3550 100644 --- a/lib/test-adapter/gemini-test-adapter.js +++ b/lib/test-adapter/gemini-test-adapter.js @@ -41,6 +41,11 @@ module.exports = class GeminiTestResultAdapter extends TestAdapter { return this._testResult.attempt; } + // for correct determine image paths in gui + set attempt(attemptNum) { + this._testResult.attempt = attemptNum; + } + get referencePath() { return this._testResult.referencePath; } diff --git a/lib/test-adapter/hermione-test-adapter.js b/lib/test-adapter/hermione-test-adapter.js index 42d61857e..657d90aca 100644 --- a/lib/test-adapter/hermione-test-adapter.js +++ b/lib/test-adapter/hermione-test-adapter.js @@ -43,6 +43,11 @@ module.exports = class HermioneTestResultAdapter extends TestAdapter { : this._config.retry; } + // for correct determine image paths in gui + set attempt(attemptNum) { + this._testResult.attempt = attemptNum; + } + get referencePath() { return _.get(this._testResult, 'err.refImagePath'); } diff --git a/test/lib/report-builder-factory/report-builder.js b/test/lib/report-builder-factory/report-builder.js index e38c560f9..dc50cd3bb 100644 --- a/test/lib/report-builder-factory/report-builder.js +++ b/test/lib/report-builder-factory/report-builder.js @@ -217,10 +217,11 @@ describe('ReportBuilder', () => { ].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}); + const test1 = stubTest_({browserId: 'bro', hasDiff: () => hasDiff}); + const test2 = stubTest_({browserId: 'bro', hasDiff: () => hasDiff}); - reportBuilder.addRetry(test); - reportBuilder.addSuccess(test); + reportBuilder.addRetry(test1); + reportBuilder.addSuccess(test2); const suiteResult = getSuiteResult_(reportBuilder); assert.equal(suiteResult.status, SUCCESS);