Merge pull request #895 from finos/better-test-archives

Better test screenshot archiving.

Author: texodus

packages/perspective-test/src/js/index.js

@@ -46,6 +46,7 @@ exports.with_server = function with_server({paths}, body) {
 };
 
 let results;
+const seen_results = new Set();
 
 const new_results = {};
 
@@ -179,10 +180,7 @@ function mkdirSyncRec(targetDir) {
 describe.page = (url, body, {reload_page = true, name, root} = {}) => {
     let _url = url ? url : page_url;
     test_root = root ? root : test_root;
-    const dir_name = path.join(test_root, "screenshots", RESULTS_TAGNAME, _url.replace(".html", ""));
-    if (!fs.existsSync(dir_name)) {
-        mkdirSyncRec(dir_name);
-    }
+
     describe(name ? name : _url, () => {
         let old = page_url;
         let old_reload = page_reload;
@@ -243,7 +241,7 @@ expect.extend({
 test.capture = function capture(name, body, {timeout = 60000, viewport = null, wait_for_update = true, fail_on_errors = true} = {}) {
     const _url = page_url;
     const _reload_page = page_reload;
-    test(
+    const spec = test(
         name,
         async () => {
             errors = [];
@@ -256,6 +254,15 @@ test.capture = function capture(name, body, {timeout = 60000, viewport = null, w
 
             const iterations = process.env.PSP_SATURATE ? 10 : 1;
 
+            const test_name = `${name.replace(/[ \.']/g, "_")}`;
+            const path_name = `${spec.result.fullName.replace(".html", "").replace(/[ \.']/g, "_")}`;
+            let dir_name = path.join(test_root, "screenshots", RESULTS_TAGNAME, path_name);
+            dir_name = dir_name.slice(0, dir_name.length - test_name.length - 1);
+            const filename = path.join(dir_name, test_name);
+            if (!fs.existsSync(dir_name)) {
+                mkdirSyncRec(dir_name);
+            }
+
             for (let x = 0; x < iterations; x++) {
                 if (_reload_page) {
                     await page.close();
@@ -313,9 +320,7 @@ test.capture = function capture(name, body, {timeout = 60000, viewport = null, w
                     .update(screenshot)
                     .digest("hex");
 
-                const filename = path.join(test_root, "screenshots", RESULTS_TAGNAME, `${_url.replace(".html", "")}`, `${name.replace(/ /g, "_").replace(/[\.']/g, "")}`);
-
-                if (hash === results[_url + "/" + name]) {
+                if (hash === results[path_name]) {
                     fs.writeFileSync(filename + ".png", screenshot);
                 } else {
                     fs.writeFileSync(filename + ".failed.png", screenshot);
@@ -329,20 +334,22 @@ test.capture = function capture(name, body, {timeout = 60000, viewport = null, w
                 }
 
                 if (process.env.WRITE_TESTS) {
-                    new_results[_url + "/" + name] = hash;
+                    new_results[path_name] = hash;
                 }
 
                 if (process.env.PSP_PAUSE_ON_FAILURE) {
-                    if (!process.env.WRITE_TESTS && (hash !== results[_url + "/" + name] || errors.length > 0)) {
+                    if (!process.env.WRITE_TESTS && (hash !== results[path_name] || errors.length > 0)) {
                         private_console.error(`Failed ${name}, pausing`);
                         await prompt(`Failed ${name}, pausing.  Press enter to continue ..`);
                     }
                 }
                 if (fail_on_errors) {
                     expect(errors).toNotError();
                 }
-                expect(hash).toBe(results[_url + "/" + name]);
+                expect(hash).toBe(results[path_name]);
+                expect(seen_results.has(path_name)).toBeFalsy();
             }
+            seen_results.add(path_name);
         },
         timeout
     );

packages/perspective-test/src/js/reporter.js

@@ -46,15 +46,23 @@ module.exports = class ImageViewerReporter {
     onTestResult(testRunConfig, testResults) {
         for (const test of testResults.testResults) {
             if (test.status === "failed") {
-                const ancestors = test.ancestorTitles.filter(x => x.indexOf(".html") > -1).map(x => x.replace(".html", "").replace(/ /g, "_"));
-                const desc = ancestors.join("/");
-                const name = test.title.replace(/ /g, "_").replace(/[\.']/g, "");
-                const filename = `${testRunConfig.path.split("/test")[0]}/screenshots/${paths.RESULTS_TAGNAME}/${desc}/${name}.diff.png`;
-                const alt_filename = `screenshots/${desc}/${name}.diff.png`;
-                if (filename) {
-                    this.write_img(test.title, ancestors, filename);
-                } else if (fs.existsSync(alt_filename)) {
-                    this.write_img(test.title, ancestors, alt_filename);
+                const name = test.title.replace(/[ \.']/g, "_");
+                let desc = test.fullName.replace(".html", "").replace(/ /g, "_");
+                desc = desc.slice(0, desc.length - name.length - 1);
+                const candidates = [
+                    `${testRunConfig.path.split("/test")[0]}/screenshots/${paths.RESULTS_TAGNAME}/${desc}/${name}.diff.png`,
+                    `screenshots/${desc}/${name}.diff.png`,
+                    `${testRunConfig.path.split("/test")[0]}/screenshots/${paths.RESULTS_TAGNAME}/${desc}/${name}.failed.png`,
+                    `screenshots/${desc}/${name}.failed.png`,
+                    `${testRunConfig.path.split("/test")[0]}/screenshots/${paths.RESULTS_TAGNAME}/${desc}/${name}.png`,
+                    `screenshots/${desc}/${name}.png`
+                ];
+
+                for (const filename of candidates) {
+                    if (fs.existsSync(filename)) {
+                        this.write_img(test.title, test.ancestorTitles, filename);
+                        break;
+                    }
                 }
             }
         }