Merge pull request #472 from gemini-testing/HERMIONE-969.reused_refs_fs

fix: dont overwrite reused references on file system

Author: KuznetsovRoman

lib/test-adapter.js

@@ -101,36 +101,43 @@ module.exports = class TestAdapter {
         return [id.toString(), browserId, stateName].join('/');
     }
 
-    _getExpectedPath(stateName, status) {
+    _getExpectedPath(stateName, status, cacheExpectedPaths) {
         const key = this._getExpectedKey(stateName);
 
         if (status === UPDATED) {
             const expectedPath = utils.getReferencePath(this, stateName);
-            globalCacheExpectedPaths.set(key, expectedPath);
+            cacheExpectedPaths.set(key, expectedPath);
 
-            return expectedPath;
+            return {path: expectedPath, reused: false};
         }
 
-        if (globalCacheExpectedPaths.has(key)) {
-            return globalCacheExpectedPaths.get(key);
+        if (cacheExpectedPaths.has(key)) {
+            return {path: cacheExpectedPaths.get(key), reused: true};
         }
 
         const imageInfo = this._getLastImageInfoFromDB(stateName);
-        const expectedPath = imageInfo && imageInfo.expectedImg
-            ? imageInfo.expectedImg.path
-            : utils.getReferencePath(this, stateName);
 
-        globalCacheExpectedPaths.set(key, expectedPath);
+        if (imageInfo && imageInfo.expectedImg) {
+            const expectedPath = imageInfo.expectedImg.path;
 
-        return expectedPath;
+            cacheExpectedPaths.set(key, expectedPath);
+
+            return {path: expectedPath, reused: true};
+        }
+
+        const expectedPath = utils.getReferencePath(this, stateName);
+
+        cacheExpectedPaths.set(key, expectedPath);
+
+        return {path: expectedPath, reused: false};
     }
 
     getImagesFor(status, stateName) {
         const refImg = this.getRefImg(stateName);
         const currImg = this.getCurrImg(stateName);
         const errImg = this.getErrImg();
 
-        const refPath = this._getExpectedPath(stateName, status);
+        const {path: refPath} = this._getExpectedPath(stateName, status, globalCacheExpectedPaths);
         const currPath = utils.getCurrentPath(this, stateName);
         const diffPath = utils.getDiffPath(this, stateName);
 
@@ -373,10 +380,10 @@ module.exports = class TestAdapter {
         await fs.writeFile(detailsFilePath, detailsData);
     }
 
-    async saveTestImages(reportPath, workers) {
+    async saveTestImages(reportPath, workers, cacheExpectedPaths = globalCacheExpectedPaths) {
         const result = await Promise.all(this.assertViewResults.map(async (assertResult) => {
             const {stateName} = assertResult;
-            const destRefPath = this._getExpectedPath(stateName);
+            const {path: destRefPath, reused: reusedReference} = this._getExpectedPath(stateName, undefined, cacheExpectedPaths);
             const srcRefPath = this.getRefImg(stateName).path;
 
             const destCurrPath = utils.getCurrentPath(this, stateName);
@@ -396,8 +403,11 @@ module.exports = class TestAdapter {
                 actions.push(
                     this._saveImg(srcCurrPath, destCurrPath, reportPath),
                     this._saveImg(srcDiffPath, dstCurrPath, reportPath),
-                    this._saveImg(srcRefPath, destRefPath, reportPath)
                 );
+
+                if (!reusedReference) {
+                    actions.push(this._saveImg(srcRefPath, destRefPath, reportPath));
+                }
             }
 
             if (this.isNoRefImageError(assertResult)) {

test/unit/hermione.js

@@ -6,6 +6,7 @@ const HermioneReporter = require('../../hermione');
 const PluginAdapter = require('lib/plugin-adapter');
 const StaticReportBuilder = require('lib/report-builder/static');
 const SqliteAdapter = require('lib/sqlite-adapter');
+const TestAdapter = require('lib/test-adapter');
 const utils = require('lib/server-utils');
 const {logger} = require('lib/common-utils');
 const {stubTool} = require('./utils');
@@ -114,6 +115,11 @@ describe('lib/hermione', () => {
 
         sandbox.stub(SqliteAdapter.prototype, 'query');
 
+        const saveTestImagesImplementation = TestAdapter.prototype.saveTestImages;
+        sandbox.stub(TestAdapter.prototype, 'saveTestImages').callsFake(function(...args) {
+            return saveTestImagesImplementation.call(this, ...args, new Map()); // disable expectedPath cache
+        });
+
         sandbox.stub(fs, 'readFile').resolves(Buffer.from(''));
     });
 

test/unit/lib/test-adapter.js

@@ -45,12 +45,12 @@ describe('hermione test adapter', () => {
         fullTitle: () => 'default-title'
     });
 
-    const mkErrStub = (ErrType = ImageDiffError) => {
+    const mkErrStub = (ErrType = ImageDiffError, {stateName, currImg, refImg} = {}) => {
         const err = new ErrType();
 
-        err.stateName = 'plain';
-        err.currImg = {path: 'curr/path'};
-        err.refImg = {path: 'ref/path'};
+        err.stateName = stateName || 'plain';
+        err.currImg = currImg || {path: 'curr/path'};
+        err.refImg = refImg || {path: 'ref/path'};
 
         return err;
     };
@@ -68,6 +68,7 @@ describe('hermione test adapter', () => {
         });
         sandbox.stub(utils, 'getCurrentPath').returns('');
         sandbox.stub(utils, 'getDiffPath').returns('');
+        sandbox.stub(utils, 'getReferencePath').returns('');
         sandbox.stub(fs, 'readFile').resolves(Buffer.from(''));
         sandbox.stub(fs, 'copy').resolves();
         err = mkErrStub();
@@ -399,6 +400,57 @@ describe('hermione test adapter', () => {
                 assert.calledWith(imagesSaver.saveImg, sinon.match('dest/path'), {destPath: 'dest/path', reportDir: 'report/path'});
             });
         });
+
+        describe('saving reference image', () => {
+            it('should save reference, if it is not reused', async () => {
+                tmp.tmpdir = 'tmp/dir';
+                const testResult = mkTestResult_({assertViewResults: [err]});
+                utils.getReferencePath.returns('ref/report/path');
+                const imagesSaver = {saveImg: sandbox.stub()};
+                const cacheExpectedPaths = new Map();
+                const hermioneTestAdapter = mkHermioneTestResultAdapter(testResult, {
+                    htmlReporter: {
+                        imagesSaver
+                    }
+                });
+
+                await hermioneTestAdapter.saveTestImages(
+                    'html-report/path',
+                    {saveDiffTo: sandbox.stub()},
+                    cacheExpectedPaths
+                );
+
+                assert.calledWith(
+                    imagesSaver.saveImg, 'ref/path',
+                    {destPath: 'ref/report/path', reportDir: 'html-report/path'}
+                );
+            });
+
+            it('should not save reference, if it is reused', async () => {
+                tmp.tmpdir = 'tmp/dir';
+                const error = mkErrStub(ImageDiffError, {stateName: 'plain'});
+                const testResult = mkTestResult_({assertViewResults: [error], browserId: 'browser-id'});
+                utils.getReferencePath.returns('ref/report/path');
+                const imagesSaver = {saveImg: sandbox.stub()};
+                const cacheExpectedPaths = new Map([['some-id/browser-id/plain', 'ref/report/path']]);
+                const hermioneTestAdapter = mkHermioneTestResultAdapter(testResult, {
+                    htmlReporter: {
+                        imagesSaver
+                    }
+                });
+
+                await hermioneTestAdapter.saveTestImages(
+                    'html-report/path',
+                    {saveDiffTo: sandbox.stub()},
+                    cacheExpectedPaths
+                );
+
+                assert.neverCalledWith(
+                    imagesSaver.saveImg, 'ref/path',
+                    {destPath: 'ref/report/path', reportDir: 'html-report/path'}
+                );
+            });
+        });
     });
 
     describe('hasDiff()', () => {
@@ -510,7 +562,7 @@ describe('hermione test adapter', () => {
     describe('getImagesInfo()', () => {
         beforeEach(() => {
             sandbox.stub(utils, 'copyFileAsync');
-            sandbox.stub(utils, 'getReferencePath').returns('some/ref.png');
+            utils.getReferencePath.returns('some/ref.png');
         });
 
         it('should not reinit "imagesInfo"', () => {