Refs #4739, make sure UI screenshots are stored in plugin directories if tests are for plugins, cleaned up path output in console and add .gitignore to DBStats plugin.

8 files changed, 126 insertions, 17 deletions

diff --git a/plugins/DBStats/.gitignore b/plugins/DBStats/.gitignore
new file mode 100644
index 0000000000..ea6abf0ffb
--- /dev/null
+++ b/plugins/DBStats/.gitignore
@@ -0,0 +1 @@
+tests/UI/processed-ui-screenshots
\ No newline at end of file
diff --git a/plugins/DBStats/tests/UI/DBStats_spec.js b/plugins/DBStats/tests/UI/DBStats_spec.js
index a3274b4bfb..253e8d36e5 100644
--- a/plugins/DBStats/tests/UI/DBStats_spec.js
+++ b/plugins/DBStats/tests/UI/DBStats_spec.js
@@ -10,7 +10,11 @@
 describe("DBStats", function () {
     this.timeout(0);
 
-    it("should pass", function () {
-        expect(1).to.be.equal(1);
+    var url = "?module=DBStats&action=index&idSite=1&period=day&date=yesterday";
+
+    it("should load correctly", function (done) {
+        expect.screenshot('admin_page').to.be.capture(function (page) {
+            page.load(url);
+        }, done);
     });
 });
\ No newline at end of file
diff --git a/tests/lib/screenshot-testing/run-tests.js b/tests/lib/screenshot-testing/run-tests.js
index 1ada102ae9..6ddde1d92f 100644
--- a/tests/lib/screenshot-testing/run-tests.js
+++ b/tests/lib/screenshot-testing/run-tests.js
@@ -8,14 +8,14 @@
  */
 
 // required modules
-var fs = require('fs'),
-    path = require('./support/path'),
-    config = require("./config");
+var config = require("./config");
+
+require('./support/fs-extras');
 
 phantom.injectJs('./support/globals.js');
 
 // make sure script works wherever it's executed from
-fs.changeWorkingDirectory(__dirname);
+require('fs').changeWorkingDirectory(__dirname);
 
 // load mocha + chai
 require('./support/mocha-loader');
@@ -27,5 +27,6 @@ if (options['help']) {
     app.printHelpAndExit();
 }
 
+app.init();
 app.loadTestModules();
 app.runTests();
\ No newline at end of file
diff --git a/tests/lib/screenshot-testing/support/app.js b/tests/lib/screenshot-testing/support/app.js
index b5d2e991cf..149c5f9a37 100644
--- a/tests/lib/screenshot-testing/support/app.js
+++ b/tests/lib/screenshot-testing/support/app.js
@@ -54,8 +54,21 @@ Application.prototype.printHelpAndExit = function () {
     phantom.exit(0);
 };
 
+Application.prototype.init = function () {
+    var app = this;
+
+    // overwrite describe function so we can inject the base directory of a suite
+    var oldDescribe = describe;
+    describe = function () {
+        var suite = oldDescribe.apply(null, arguments);
+        suite.baseDirectory = path.dirname(app.currentModulePath);
+        return suite;
+    };
+};
+
 Application.prototype.loadTestModules = function () {
-    var pluginDir = path.join(PIWIK_INCLUDE_PATH, 'plugins');
+    var self = this,
+        pluginDir = path.join(PIWIK_INCLUDE_PATH, 'plugins');
 
     // find all installed plugins
     var plugins = fs.list(pluginDir).map(function (item) {
@@ -72,6 +85,8 @@ Application.prototype.loadTestModules = function () {
     });
 
     modulePaths.forEach(function (path) {
+        self.currentModulePath = path;
+
         require(path);
     });
 
diff --git a/tests/lib/screenshot-testing/support/chai-extras.js b/tests/lib/screenshot-testing/support/chai-extras.js
index af7ebab4d6..694ed7b503 100644
--- a/tests/lib/screenshot-testing/support/chai-extras.js
+++ b/tests/lib/screenshot-testing/support/chai-extras.js
@@ -50,10 +50,10 @@ chai.Assertion.addChainableMethod('capture', function () {
         throw new Error("No 'done' callback specified in capture assertion.");
     }
 
-    // TODO: tests dir should depend on module path
     var screenshotFileName = screenName + '.png',
-        expectedScreenshotPath = path.join(uiTestsDir, config.expectedScreenshotsDir, compareAgainst + '.png'),
-        processedScreenshotPath = path.join(uiTestsDir, config.processedScreenshotsDir, screenshotFileName);
+        dirsBase = app.runner.suite.baseDirectory,
+        expectedScreenshotPath = path.join(dirsBase, config.expectedScreenshotsDir, compareAgainst + '.png'),
+        processedScreenshotPath = path.join(dirsBase, config.processedScreenshotsDir, screenshotFileName);
 
     pageSetupFn(pageRenderer);
 
@@ -85,11 +85,14 @@ chai.Assertion.addChainableMethod('capture', function () {
             var fail = function (message) {
                 app.diffViewerGenerator.failures.push(testInfo);
 
+                var expectedPath = testInfo.expected ? path.resolve(testInfo.expected) : "",
+                    processedPath = testInfo.processed ? path.resolve(testInfo.processed) : "";
+
                 var indent = "     ";
                 var failureInfo = message + "\n";
                 failureInfo += indent + "Url to reproduce: " + pageRenderer.getCurrentUrl() + "\n";
-                failureInfo += indent + "Generated screenshot: " + testInfo.processed + "\n";
-                failureInfo += indent + "Expected screenshot: " + testInfo.expected + "\n";
+                failureInfo += indent + "Generated screenshot: " + processedPath + "\n";
+                failureInfo += indent + "Expected screenshot: " + expectedPath + "\n";
                 failureInfo += indent + "Screenshot diff: " + app.diffViewerGenerator.getDiffPath(testInfo);
 
                 failureInfo += getPageLogsString(pageRenderer.pageLogs, indent);
diff --git a/tests/lib/screenshot-testing/support/diff-viewer.js b/tests/lib/screenshot-testing/support/diff-viewer.js
index 5cbee27784..738f428856 100644
--- a/tests/lib/screenshot-testing/support/diff-viewer.js
+++ b/tests/lib/screenshot-testing/support/diff-viewer.js
@@ -7,6 +7,9 @@
  * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
  */
 
+var fs = require('fs'),
+    path = require('./path');
+
 var DiffViewerGenerator = function (diffDir) {
     this.diffDir = diffDir;
     this.outputPath = path.join(diffDir, 'diffviewer.html');
@@ -30,7 +33,12 @@ DiffViewerGenerator.prototype.checkImageMagickCompare = function (callback) {
 };
 
 DiffViewerGenerator.prototype.getDiffPath = function (testInfo) {
-    return path.join(this.diffDir, testInfo.name + '.png');
+    return path.resolve(path.join(this.diffDir, testInfo.name + '.png'));
+};
+
+// TODO: diff output path shouldn't be stored in piwik-ui-tests repo
+DiffViewerGenerator.prototype.getUrlForPath = function (path) {
+    return fs.relpath(path, this.diffDir);
 };
 
 DiffViewerGenerator.prototype.generate = function (callback) {
@@ -58,18 +66,24 @@ DiffViewerGenerator.prototype.generate = function (callback) {
             var entry = self.failures[i];
 
             if (entry.expected) {
-                entry.expectedUrl = 'https://raw.github.com/piwik/piwik-ui-tests/master/expected-ui-screenshots/'
-                                  + entry.name + '.png';
+                var expectedUrl = self.getUrlForPath(entry.expected);
+                var expectedUrlGithub = 'https://raw.github.com/piwik/piwik-ui-tests/master/expected-ui-screenshots/'
+                                      + entry.name + '.png';
+
+                var expectedHtml = '<a href="' + entry.expectedUrl + '">Expected</a>&nbsp;<a href="' + expectedUrlGithub
+                                 + '">[Github]</a>';
+            } else {
+                var expectedHtml = '<em>Not found</em>';
             }
 
             if (entry.processed) {
-                entry.processedUrl = '../processed-ui-screenshots/' + entry.name + '.png';
+                entry.processedUrl = self.getUrlForPath(entry.processed);
             }
 
             diffViewerContent += '\
     <tr>\
         <td>' + entry.name + '</td>\
-        <td>' + (entry.expected ? ('<a href="' + entry.expectedUrl + '">Expected</a>') : '<em>Not found</em>') + '</td>\
+        <td>' + expectedHtml + '</td>\
         <td>' + (entry.processed ? ('<a href="' + entry.processedUrl + '">Processed</a>') : '<em>Not found</em>') + '</td>\
         <td>' + (entry.diffUrl ? ('<a href="' + entry.diffUrl + '">Difference</a>') : '<em>Could not create diff.</em>') + '</td>\
     </tr>';
diff --git a/tests/lib/screenshot-testing/support/fs-extras.js b/tests/lib/screenshot-testing/support/fs-extras.js
new file mode 100644
index 0000000000..665d91401f
--- /dev/null
+++ b/tests/lib/screenshot-testing/support/fs-extras.js
@@ -0,0 +1,45 @@
+var fs = require('fs'),
+    path = require('./path');
+
+fs.commonprefixLength = function (lists) {
+    var l = 0;
+
+    var rest = lists.slice(1);
+
+    while (l < lists[0].length) {
+        for (var i = 0; i != rest.length; ++i) {
+            var list = rest[i];
+            if (l == list.length
+                || list[l] != lists[0][l]
+            ) {
+                return l;
+            }
+        }
+
+        l += 1;
+    }
+
+    return l;
+};
+
+// This and the helper function above are essentially the python version ported to JavaScript
+fs.relpath = function (p, start) {
+    start_list = path.resolve(start).substring(1).split('/');
+    path_list = path.resolve(p).substring(1).split('/');
+
+    l = fs.commonprefixLength([start_list, path_list]);
+
+    rel_list = []
+    for (var i = 0; i < start_list.length - l; ++i) {
+        rel_list.push('..');
+    }
+
+    rel_list.push.apply(rel_list, path_list.slice(l));
+
+
+    if (rel_list.length == 0) {
+        return '.';
+    }
+
+    return path.join.apply(null, rel_list)
+};
\ No newline at end of file
diff --git a/tests/lib/screenshot-testing/support/path.js b/tests/lib/screenshot-testing/support/path.js
index f9f4b7df70..2d5e796f1d 100644
--- a/tests/lib/screenshot-testing/support/path.js
+++ b/tests/lib/screenshot-testing/support/path.js
@@ -10,3 +10,29 @@
 exports.join = function () {
     return Array.prototype.join.call(arguments, "/").replace(/[\\\/]{2,}/g, "/");
 };
+
+exports.dirname = function (path) {
+    var lastSeparator = path.lastIndexOf("/");
+    return lastSeparator == -1 ? path : path.substring(0, lastSeparator);
+};
+
+exports.resolve = function (path) {
+    if (path.charAt(0) != '/') {
+        path = exports.join(__dirname, path);
+    }
+
+    var path_split = path.split('/'),
+        result = [];
+
+    for (var i = 0; i != path_split.length; ++i) {
+        if (path_split[i] == '.') {
+            continue;
+        } else if (path_split[i] == '..') {
+            result.pop();
+        } else {
+            result.push(path_split[i]);
+        }
+    }
+
+    return exports.join.apply(exports, result);
+};
\ No newline at end of file