jenkins-bot has submitted this change and it was merged.
Change subject: Flag changed diffs in rt-server results views
......................................................................
Flag changed diffs in rt-server results views
* This patch adds new views; previous result views are still available.
* The new commit link in the regressions/fixes view now takes you to
a results view that flags skips/fails occuring in the new commit
but not in the old commit. And the old commit link takes you to a
results view that flags skips/fails no longer in the new commit.
* Overflagging (eg flagging too many diffs as new) is possible.
Say a page edit changes lines in a way that we would think of the
"same diff" occurring in "different lines." Then it's technically a
new line-based diff, and will be flagged as such. This does not seem
to occur too much in practice (and won't happen at all if, in the
future, results are pinned to a specific page edit across test runs).
Change-Id: I95dafb77e83ad63b7f46de25d48f78ba19fd0704
---
M lib/mediawiki.Util.js
A tests/server/diff.js
M tests/server/package.json
M tests/server/server.js
M tests/server/static/result.css
5 files changed, 241 insertions(+), 17 deletions(-)
Approvals:
Marcoil: Looks good to me, approved
jenkins-bot: Verified
diff --git a/lib/mediawiki.Util.js b/lib/mediawiki.Util.js
index f04ed86..8066754 100644
--- a/lib/mediawiki.Util.js
+++ b/lib/mediawiki.Util.js
@@ -1293,6 +1293,14 @@
return pairs;
};
+var diffTokens = function(oldString, newString, tokenize) {
+ if (oldString === newString) {
+ return [['=', [newString]]];
+ } else {
+ return simpleDiff.diff(tokenize(oldString),
tokenize(newString));
+ }
+};
+
var diffLines = function(oldString, newString) {
var lineTokenize = function(value) {
var retLines = [],
@@ -1306,17 +1314,11 @@
} else if (line) {
retLines.push(line);
}
- }
+ }
return retLines;
};
-
- if (oldString === newString) {
- return [['=', [newString]]];
- } else {
- return simpleDiff.diff(lineTokenize(oldString),
lineTokenize(newString));
- }
+ return diffTokens(oldString, newString, lineTokenize);
};
-
Util.convertDiffToOffsetPairs = convertDiffToOffsetPairs;
Util.diffLines = diffLines;
diff --git a/tests/server/diff.js b/tests/server/diff.js
new file mode 100644
index 0000000..b532dcc
--- /dev/null
+++ b/tests/server/diff.js
@@ -0,0 +1,98 @@
+/*
+* This file contains diff related functions for use in tests/server,
+* to compare test results for a given page from different revisions.
+*/
+
+"use strict";
+var simpleDiff = require('simplediff');
+
+var Diff = {};
+( function (exports) {
+
+ var diffTokens = function(oldString, newString, tokenize) {
+ if (oldString === newString) {
+ return [['=', [newString]]];
+ } else {
+ return simpleDiff.diff(tokenize(oldString),
tokenize(newString));
+ }
+ };
+
+ var diffResults = function(oldString, newString) {
+ var testcaseTokenize = function(resultString) {
+ var testcases =
resultString.split(/<\/skipped>|<\/failure>/);
+ // Omit everything that's not part of a <skipped> or
<failure> element,
+ // as this can contain info we're not interested in
diffing
+ // (eg character number within original text,
perfstats).
+ testcases = testcases.slice(0, -1);
+ testcases = testcases.map(function(testcase) {
+ var skipTagIndex = testcase.indexOf('<skipped');
+ if (skipTagIndex !== -1) {
+ return testcase.slice(skipTagIndex);
+ } else {
+ return
testcase.slice(testcase.indexOf('<failure'));
+ }
+ });
+ return testcases;
+ };
+
+ return diffTokens(oldString, newString, testcaseTokenize);
+ };
+
+ var testcaseStatus = function(diff, flag) {
+ // Returns an array of 0's and 1's, where, supposing flag is
'+', 1 in the nth position
+ // means that the n'th token of the newer diffed item isn't a
token of the older item.
+ // (And symmetrically for '-', interchanging roles of 'newer'
and 'older'.)
+ var array = [];
+ for (var i = 0, l = diff.length; i < l; i++) {
+ var change = diff[i];
+ if (change[0] === flag) {
+ for (var j = 0; j < change[1].length; j++) {
+ array.push(1);
+ }
+ } else if (change[0] === '=') {
+ for (var k = 0; k < change[1].length; k++) {
+ array.push(0);
+ }
+ }
+ }
+ return array;
+ };
+
+ // If flag is '+', adds status="new" attribute to <testcase> tags for
testcases ocurring in
+ // newString but not in oldString. Otherwise (flag is '-') adds
status="old" to <testcase>'s occuring in
+ // oldString but not in newString.
+ exports.resultFlagged = function(oldString, newString, oldCommit,
newCommit, flag) {
+ // If one of the two results is an error, don't flag
differences.
+ if (oldString.slice(0,6) === '<error' || newString.slice(0,6)
=== '<error') {
+ var output = flag === '+' ? newString : oldString;
+ return output;
+ }
+
+ var status = flag === '+' ? 'new' : 'old';
+ var xmlWrapper = flag === '+' ? 'FlagNewTestcases' :
'FlagOldTestcases';
+ var testcases = flag === '+' ?
newString.split(/(<\/testcase>)/) : oldString.split(/(<\/testcase>)/);
+
+ var diff = diffResults(oldString, newString);
+ var statusArray = testcaseStatus(diff, flag);
+ var startTestcases = testcases[0].indexOf('<testcase');
+ var pre = testcases[0].slice(0, startTestcases);
+ var post = testcases[testcases.length - 1];
+ testcases[0] = testcases[0].slice(startTestcases);
+
+ var results = [];
+ for (var i = 0, l = testcases.length - 1; i < l; i++) {
+ if (i%2 === 0 && statusArray[i/2]) {
+ testcases[i] =
testcases[i].replace('<testcase', '<testcase status="' + status + '"');
+ }
+ results.push(testcases[i]);
+ }
+ var result = results.join('');
+ return '<' + xmlWrapper + ' oldCommit ="' + oldCommit + '"
newCommit="' + newCommit + '" >' +
+ pre + result + post + '</' + xmlWrapper + '>';
+ };
+
+}(Diff));
+
+if (typeof module === "object") {
+ module.exports.Diff = Diff;
+}
\ No newline at end of file
diff --git a/tests/server/package.json b/tests/server/package.json
index 79560a1..02eac44 100644
--- a/tests/server/package.json
+++ b/tests/server/package.json
@@ -10,7 +10,8 @@
"mysql": "2.x.x",
"mysql-queues": "1.x.x",
"express": "2.5.x",
- "handlebars": "~1.3.0"
+ "handlebars": "~1.3.0",
+ "simplediff": "0.1.1"
},
"main": "server.js"
}
diff --git a/tests/server/server.js b/tests/server/server.js
index 8b5ee33..3b053a5 100755
--- a/tests/server/server.js
+++ b/tests/server/server.js
@@ -4,7 +4,8 @@
var express = require( 'express' ),
optimist = require( 'optimist' ),
- hbs = require( 'handlebars' );
+ hbs = require( 'handlebars' ),
+ Diff = require('./diff.js').Diff;
// Default options
var defaults = {
@@ -328,6 +329,14 @@
'SELECT result FROM results ' +
'JOIN pages ON pages.id = results.page_id ' +
'WHERE results.commit_hash = ? AND pages.title = ? AND pages.prefix = ?';
+
+var dbGetTwoResults =
+ 'SELECT result FROM results ' +
+ 'JOIN commits ON results.commit_hash = commits.hash ' +
+ 'JOIN pages ON pages.id = results.page_id ' +
+ 'WHERE pages.title = ? AND pages.prefix = ? ' +
+ 'AND (commits.hash = ? OR commits.hash = ?) ' +
+ 'ORDER BY commits.timestamp';
var dbFailedFetches =
'SELECT title, prefix FROM pages WHERE num_fetch_errors >= ?';
@@ -941,7 +950,6 @@
return Math.round( val * 100 ) / 100;
}
});
-
res.render('index.html', data);
}
});
@@ -1026,6 +1034,46 @@
} else {
db.query( dbGetOneResult, [ title, prefix ],
resultWebCallback.bind( null, req, res ) );
}
+};
+
+var diffResultWebCallback = function(req, res, flag, err, row) {
+ if ( err ) {
+ console.error( err );
+ res.send( err.toString(), 500 );
+ } else if (row.length === 2) {
+ var oldCommit = req.params[0].slice(0,10);
+ var newCommit = req.params[1].slice(0,10);
+ var oldResult = row[0].result;
+ var newResult = row[1].result;
+ var flagResult = Diff.resultFlagged(oldResult, newResult,
oldCommit, newCommit, flag);
+ res.setHeader( 'Content-Type', 'text/xml; charset=UTF-8' );
+ res.status(200);
+ res.write( '<?xml-stylesheet href="/static/result.css"?>\n' );
+ res.end(flagResult);
+ } else {
+ var commit = flag === '+' ? req.params[1] : req.params[0];
+ res.redirect('/result/' + commit + '/' + req.params[2] + '/' +
req.params[3]);
+ }
+};
+
+var resultFlagNewWebInterface = function(req, res) {
+ var oldCommit = req.params[0];
+ var newCommit = req.params[1];
+ var prefix = req.params[2];
+ var title = req.params[3];
+
+ db.query(dbGetTwoResults, [ title, prefix, oldCommit, newCommit ],
+ diffResultWebCallback.bind(null, req, res, '+'));
+};
+
+var resultFlagOldWebInterface = function(req, res) {
+ var oldCommit = req.params[0];
+ var newCommit = req.params[1];
+ var prefix = req.params[2];
+ var title = req.params[3];
+
+ db.query(dbGetTwoResults, [ title, prefix, oldCommit, newCommit ],
+ diffResultWebCallback.bind(null, req, res, '-'));
};
var GET_failedFetches = function( req, res ) {
@@ -1146,12 +1194,26 @@
};
};
+var newCommitLinkData = function(oldCommit, newCommit, title, prefix) {
+ return {
+ url: '/resultFlagNew/' + oldCommit + '/' + newCommit + '/' +
prefix + '/' + title,
+ name: newCommit.substr(0,7)
+ };
+};
+
+var oldCommitLinkData = function(oldCommit, newCommit, title, prefix) {
+ return {
+ url: '/resultFlagOld/' + oldCommit + '/' + newCommit + '/' +
prefix + '/' + title,
+ name: oldCommit.substr(0,7)
+ };
+};
+
var makeRegressionRow = function(row) {
return [
pageTitleData(row),
- commitLinkData(row.old_commit, row.title, row.prefix),
+ oldCommitLinkData(row.old_commit, row.new_commit, row.title,
row.prefix),
row.old_errors + "|" + row.old_fails + "|" + row.old_skips,
- commitLinkData(row.new_commit, row.title, row.prefix),
+ newCommitLinkData(row.old_commit, row.new_commit, row.title,
row.prefix),
row.errors + "|" + row.fails + "|" + row.skips
];
};
@@ -1159,8 +1221,8 @@
var makeOneDiffRegressionRow = function(row) {
return [
pageTitleData(row),
- commitLinkData(row.old_commit, row.title, row.prefix),
- commitLinkData(row.new_commit, row.title, row.prefix)
+ oldCommitLinkData(row.old_commit, row.new_commit, row.title,
row.prefix),
+ newCommitLinkData(row.old_commit, row.new_commit, row.title,
row.prefix)
];
};
@@ -1514,6 +1576,12 @@
// Results for a title on any commit
app.get( /^\/result\/([a-f0-9]*)\/([^\/]+)\/(.*)$/, resultWebInterface );
+// Results for a title on a commit, flag skips/fails new since older commit
+app.get( /^\/resultFlagNew\/([a-f0-9]*)\/([a-f0-9]*)\/([^\/]+)\/(.*)$/,
resultFlagNewWebInterface );
+
+// Results for a title on a commit, flag skips/fails no longer in newer commit
+app.get( /^\/resultFlagOld\/([a-f0-9]*)\/([a-f0-9]*)\/([^\/]+)\/(.*)$/,
resultFlagOldWebInterface );
+
// List of failures sorted by severity
app.get( /^\/topfails\/(\d+)$/, failsWebInterface );
// 0th page
diff --git a/tests/server/static/result.css b/tests/server/static/result.css
index 8661c53..a914d45 100644
--- a/tests/server/static/result.css
+++ b/tests/server/static/result.css
@@ -4,11 +4,11 @@
display: block;
border-left: 7px solid red;
}
+
skipped {
font-family: monospace;
white-space: pre;
display: block;
- /* orange border */
border-left: 7px solid yellow;
}
@@ -20,8 +20,63 @@
testcase:before {
font-size: 70%;
font-weight: bold;
- color: grey;
+ color: gray;
margin-left: 8px;
content: attr(name) ":";
}
+/* For result comparison views (flag changed skips/fails) */
+
+FlagNewTestcases:before {
+ padding-top: 8px;
+ padding-left: 8px;
+ margin-bottom: 8px;
+ display: block;
+ font-size: 1.2em;
+ border-bottom: 1px solid #aaa;
+ content: "Diffs in commit " attr(newCommit) " (flagged if new since older
" attr(oldCommit) ")";
+ font-family: sans-serif;
+}
+
+FlagOldTestcases:before {
+ padding-top: 8px;
+ padding-left: 8px;
+ margin-bottom: 8px;
+ display: block;
+ font-size: 1.2em;
+ border-bottom: 1px solid #aaa;
+ content: "Diffs in commit " attr(oldCommit) " (flagged if no longer in
newer " attr(newCommit) ")";
+ font-family: sans-serif;
+}
+
+testcase[status='new'] skipped:before {
+ background: yellow;
+ font-size: 70%;
+ content: 'New Syntactic Diff ';
+ font-family: sans-serif;
+ padding: 2px;
+}
+
+testcase[status='new'] failure:before {
+ background: red;
+ font-size: 70%;
+ content: 'New Semantic Diff ';
+ font-family: sans-serif;
+ padding: 2px;
+}
+
+testcase[status='old'] skipped:before {
+ background-color: rgba(255,255,0,0.2);
+ font-size: 70%;
+ content: 'Not in newer revision ';
+ font-family: sans-serif;
+ padding: 2px;
+}
+
+testcase[status='old'] failure:before {
+ background-color: rgba(255,0,0,0.2);
+ font-size: 70%;
+ content: 'Not in newer revision ';
+ font-family: sans-serif;
+ padding: 2px;
+}
\ No newline at end of file
--
To view, visit https://gerrit.wikimedia.org/r/117116
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I95dafb77e83ad63b7f46de25d48f78ba19fd0704
Gerrit-PatchSet: 5
Gerrit-Project: mediawiki/services/parsoid
Gerrit-Branch: master
Gerrit-Owner: Bebirchall <bebirch...@gmail.com>
Gerrit-Reviewer: Bebirchall <bebirch...@gmail.com>
Gerrit-Reviewer: Marcoil <marc...@wikimedia.org>
Gerrit-Reviewer: Subramanya Sastry <ssas...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
