Merge pull request #536 from gemini-testing/HERMIONE-1412.fix_ref_upd…

…ate_status fix: use propper result status after reference update
gemini-testing · Apr 2, 2024 · 8c95d9c · 8c95d9c
2 parents e752653 + f617ca6
commit 8c95d9c
Show file tree

Hide file tree

Showing 4 changed files with 74 additions and 8 deletions.
diff --git a/lib/gui/tool-runner/index.ts b/lib/gui/tool-runner/index.ts
@@ -194,13 +194,15 @@ export class ToolRunner {
 
         return Promise.all(tests.map(async (test): Promise<TestBranch> => {
             const updateResult = this._createHermioneTestResult(test);
+            const currentResult = formatTestResult(updateResult, UPDATED, test.attempt);
+            const estimatedStatus = reportBuilder.getUpdatedReferenceTestStatus(currentResult);
 
             const formattedResultWithoutAttempt = formatTestResult(updateResult, UPDATED);
             const formattedResult = reportBuilder.provideAttempt(formattedResultWithoutAttempt);
 
             const formattedResultUpdated = await reporterHelper.updateReferenceImages(formattedResult, this._reportPath, this._handleReferenceUpdate.bind(this));
 
-            await reportBuilder.addTestResult(formattedResultUpdated);
+            await reportBuilder.addTestResult(formattedResultUpdated, {status: estimatedStatus});
 
             return reportBuilder.getTestBranch(formattedResultUpdated.id);
         }));

diff --git a/lib/report-builder/gui.ts b/lib/report-builder/gui.ts
@@ -1,12 +1,12 @@
 import _ from 'lodash';
 import {StaticReportBuilder, StaticReportBuilderOptions} from './static';
 import {GuiTestsTreeBuilder, TestBranch, TestEqualDiffsData, TestRefUpdateData} from '../tests-tree-builder/gui';
-import {UPDATED, DB_COLUMNS, ToolName, HERMIONE_TITLE_DELIMITER, SKIPPED} from '../constants';
+import {UPDATED, DB_COLUMNS, ToolName, TestStatus, HERMIONE_TITLE_DELIMITER, SKIPPED, SUCCESS} from '../constants';
 import {ConfigForStaticFile, getConfigForStaticFile} from '../server-utils';
 import {ReporterTestResult} from '../test-adapter';
 import {Tree, TreeImage} from '../tests-tree-builder/base';
 import {ImageInfoFull, ImageInfoWithState, ReporterConfig} from '../types';
-import {isUpdatedStatus} from '../common-utils';
+import {determineStatus, isUpdatedStatus} from '../common-utils';
 import {HtmlReporter, HtmlReporterValues} from '../plugin-api';
 import {SkipItem} from '../tests-tree-builder/static';
 import {copyAndUpdate} from '../test-adapter/utils';
@@ -134,7 +134,25 @@ export class GuiReportBuilder extends StaticReportBuilder {
         return {updatedImage, removedResult, previousExpectedPath, shouldRemoveReference, shouldRevertReference, newResult};
     }
 
-    override async addTestResult(formattedResultOriginal: ReporterTestResult): Promise<ReporterTestResult> {
+    getUpdatedReferenceTestStatus(testResult: ReporterTestResult): TestStatus {
+        const getStateName = (imageInfo: ImageInfoFull): string => (imageInfo as ImageInfoWithState).stateName;
+        const resultId = testResult.id;
+        const originalResult = this._testsTree.getTestBranch(resultId);
+        const omittedImageStates = testResult.imagesInfo.map(getStateName);
+
+        const estimatedStatus = determineStatus({
+            status: originalResult.result.status,
+            error: originalResult.result.error,
+            imagesInfo: originalResult.images.filter(image => !omittedImageStates.includes(getStateName(image)))
+        });
+
+        return estimatedStatus === SUCCESS ? UPDATED : estimatedStatus;
+    }
+
+    override async addTestResult(
+        formattedResultOriginal: ReporterTestResult,
+        updates?: Partial<ReporterTestResult>
+    ): Promise<ReporterTestResult> {
         const formattedResult = await super.addTestResult(formattedResultOriginal);
 
         if (formattedResult.status === SKIPPED) {
@@ -148,10 +166,13 @@ export class GuiReportBuilder extends StaticReportBuilder {
         }
 
         const formattedResultWithImages = this._loadImagesFromPreviousAttempt(formattedResult);
+        const resultOverrided = updates
+            ? copyAndUpdate(formattedResultWithImages, updates)
+            : formattedResultWithImages;
 
-        this._testsTree.addTestResult(formattedResultWithImages);
+        this._testsTree.addTestResult(resultOverrided);
 
-        return formattedResultWithImages;
+        return resultOverrided;
     }
 
     private _loadImagesFromPreviousAttempt(formattedResult: ReporterTestResult): ReporterTestResult {

diff --git a/lib/reporter-helpers.ts b/lib/reporter-helpers.ts
@@ -28,8 +28,7 @@ export const updateReferenceImages = async (testResult: ReporterTestResult, repo
             ? path.resolve(reportPath, actualImg.path)
             : utils.getCurrentAbsolutePath(testResult, reportPath, stateName);
 
-        // TODO: get rid of type assertion
-        const referencePath = newImageInfo?.refImg?.path as string;
+        const referencePath = newImageInfo.refImg.path;
 
         if (utils.fileExists(referencePath)) {
             const referenceId = mkReferenceHash(testResult.id, stateName);

diff --git a/test/unit/lib/report-builder/gui.js b/test/unit/lib/report-builder/gui.js
@@ -155,6 +155,26 @@ describe('GuiReportBuilder', () => {
                 {stateName: 'state-2', status: UPDATED}
             ]);
         });
+
+        it('should load images info from previous attempt, while being overrided with fail status', async () => {
+            const reportBuilder = await mkGuiReportBuilder_();
+            GuiTestsTreeBuilder.prototype.getImagesInfo.returns([
+                {stateName: 'state-1', status: ERROR},
+                {stateName: 'state-2', status: ERROR}
+            ]);
+
+            copyAndUpdate.callsFake(_.assign);
+            const enrichedResult = await reportBuilder.addTestResult(stubTest_({
+                status: UPDATED,
+                imagesInfo: [{stateName: 'state-2', status: UPDATED}]
+            }), {status: FAIL});
+
+            assert.equal(enrichedResult.status, FAIL);
+            assert.deepEqual(enrichedResult.imagesInfo, [
+                {stateName: 'state-1', status: ERROR},
+                {stateName: 'state-2', status: UPDATED}
+            ]);
+        });
     });
 
     describe('"setApiValues" method', () => {
@@ -375,6 +395,30 @@ describe('GuiReportBuilder', () => {
         });
     });
 
+    describe('"getUpdatedReferenceTestStatus"', () => {
+        it('should return estimated by determineStatus status', async () => {
+            const reportBuilder = await mkGuiReportBuilder_();
+            const testResult = {id: 'result-id', imagesInfo: [{stateName: 'foo'}]};
+            const testBranch = {result: {status: 'fail', error: 'some-error'}, images: [{stateName: 'foo'}, {stateName: 'bar'}]};
+            GuiTestsTreeBuilder.prototype.getTestBranch.withArgs('result-id').returns(testBranch);
+
+            const estimatedStatus = reportBuilder.getUpdatedReferenceTestStatus(testResult);
+
+            assert.equal(estimatedStatus, ERROR);
+        });
+
+        it('should return "updated", if there are no errors', async () => {
+            const reportBuilder = await mkGuiReportBuilder_();
+            const testResult = {id: 'result-id', imagesInfo: [{stateName: 'foo'}]};
+            const testBranch = {result: {status: 'fail'}, images: [{stateName: 'foo'}]};
+            GuiTestsTreeBuilder.prototype.getTestBranch.withArgs('result-id').returns(testBranch);
+
+            const estimatedStatus = reportBuilder.getUpdatedReferenceTestStatus(testResult);
+
+            assert.equal(estimatedStatus, UPDATED);
+        });
+    });
+
     describe('add test result to tree', () => {
         describe('should pass test result with', () => {
             it('"suiteUrl" field', async () => {