Teach dashboard code to compare non-integer revisions
authorjmarcell@apple.com <jmarcell@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 11 Feb 2016 00:29:24 +0000 (00:29 +0000)
committerjmarcell@apple.com <jmarcell@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 11 Feb 2016 00:29:24 +0000 (00:29 +0000)
https://bugs.webkit.org/show_bug.cgi?id=152345

Reviewed by Daniel Bates.

* BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotQueue.js:
(BuildbotQueue.prototype.compareIterationsByRevisions): Compare non-integer revisions.
* BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotQueueView.js:
(BuildbotQueueView.prototype._appendPendingRevisionCount): Use Trac.indexOfRevision in order to compare non-integer
revisions. Also uses new Trac.commitsOnBranchLaterThanRevision method.
(BuildbotQueueView.prototype._popoverLinesForCommitRange): Ditto.
(BuildbotQueueView.prototype._presentPopoverForPendingCommits): Use Trac.indexOfRevision in order to compare non-integer
revisions. Also uses new Trac.nextRevision method to calculate a revision range.
(BuildbotQueueView.prototype._revisionContentWithPopoverForIteration): Ditto.
* BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/Trac.js:
(Trac.prototype._commitsOnBranch): Renamed this to indicate that it should be a private method used by the latter two new
methods.
(Trac.prototype.commitsOnBranchLaterThanRevision): Finds revisions on a branch later than the specified revision.
(Trac.prototype.commitsOnBranchInRevisionRange): Finds revisions on a branch within a specified range.
(Trac.prototype.nextRevision): Finds the next revision after a given revision on a specific branch.
(Trac.prototype.indexOfRevision): Finds the index of a given revision within the recordedCommits array.
(Trac.prototype.commitsOnBranch): Deleted. Renamed to _commitsOnBranch.
* BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/tests/tests.js: Added unit tests.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@196402 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotQueue.js
Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotQueueView.js
Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/Trac.js
Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/tests/tests.js
Tools/ChangeLog

index 77198d0..229ab86 100644 (file)
@@ -285,9 +285,15 @@ BuildbotQueue.prototype = {
         var sortedRepositories = Dashboard.sortedRepositories;
         for (var i = 0; i < sortedRepositories.length; ++i) {
             var repositoryName = sortedRepositories[i].name;
-            var result = b.revision[repositoryName] - a.revision[repositoryName];
-            if (result)
-                return result;
+            var trac = sortedRepositories[i].trac;
+            console.assert(trac);
+            var indexA = trac.indexOfRevision(a.revision[repositoryName]);
+            var indexB = trac.indexOfRevision(b.revision[repositoryName]);
+            if (indexA !== -1 && indexB !== -1) {
+                var result = indexB - indexA;
+                if (result)
+                    return result;
+            }
         }
 
         return 0;
index e17fb63..92b829c 100644 (file)
@@ -94,12 +94,12 @@ BuildbotQueueView.prototype = {
                 continue;
             if (!trac)
                 continue;
-            if (!trac.latestRecordedRevisionNumber || trac.oldestRecordedRevisionNumber > latestProductiveRevisionNumber) {
+            if (!trac.latestRecordedRevisionNumber || trac.indexOfRevision(trac.oldestRecordedRevisionNumber) > trac.indexOfRevision(latestProductiveRevisionNumber)) {
                 trac.loadMoreHistoricalData();
                 return;
             }
 
-            totalRevisionsBehind += trac.commitsOnBranch(branch.name, function(commit) { return commit.revisionNumber > latestProductiveRevisionNumber; }).length;
+            totalRevisionsBehind += trac.commitsOnBranchLaterThanRevision(branch.name, latestProductiveRevisionNumber).length;
         }
 
         if (!totalRevisionsBehind)
@@ -140,11 +140,11 @@ BuildbotQueueView.prototype = {
             return result;
         }
 
-        console.assert(trac.oldestRecordedRevisionNumber <= firstRevisionNumber);
+        console.assert(trac.indexOfRevision(trac.oldestRecordedRevisionNumber) <= trac.indexOfRevision(firstRevisionNumber));
 
         // FIXME: To be 100% correct, we should also filter out changes that are ignored by
         // the queue, see _should_file_trigger_build in wkbuild.py.
-        var commits = trac.commitsOnBranch(branch.name, function(commit) { return commit.revisionNumber >= firstRevisionNumber && commit.revisionNumber <= lastRevisionNumber; });
+        var commits = trac.commitsOnBranchInRevisionRange(branch.name, firstRevisionNumber, lastRevisionNumber);
         return commits.map(function(commit) {
             return lineForCommit.call(this, trac, commit);
         }, this).reverse();
@@ -169,7 +169,10 @@ BuildbotQueueView.prototype = {
             var latestProductiveRevisionNumber = latestProductiveIteration.revision[repositoryName];
             if (!latestProductiveRevisionNumber || !trac.latestRecordedRevisionNumber)
                 continue;
-            var lines = this._popoverLinesForCommitRange(trac, branch, latestProductiveRevisionNumber + 1, trac.latestRecordedRevisionNumber);
+            var nextRevision = trac.nextRevision(branch.name, latestProductiveRevisionNumber);
+            if (nextRevision === Trac.NO_MORE_REVISIONS)
+                continue;
+            var lines = this._popoverLinesForCommitRange(trac, branch, nextRevision, trac.latestRecordedRevisionNumber);
             var length = lines.length;
             if (length && shouldAddDivider)
                 this._addDividerToPopover(content);
@@ -220,21 +223,41 @@ BuildbotQueueView.prototype = {
         var repository = branch.repository;
         var repositoryName = repository.name;
         console.assert(iteration.revision[repositoryName]);
+        var trac = repository.trac;
         var content = document.createElement("span");
         content.textContent = this._formatRevisionForDisplay(iteration.revision[repositoryName], repository);
         content.classList.add("revision-number");
 
-        if (previousIteration) {
-            console.assert(previousIteration.revision[repositoryName]);
-            var context = {
-                trac: repository.trac,
-                branch: branch,
-                firstRevision: previousIteration.revision[repositoryName] + 1,
-                lastRevision: iteration.revision[repositoryName]
-            };
-            if (context.firstRevision <= context.lastRevision)
-                new PopoverTracker(content, this._presentPopoverForRevisionRange.bind(this), context);
-        }
+        if (!previousIteration)
+            return content;
+
+        var previousIterationRevision = previousIteration.revision[repositoryName];
+        console.assert(previousIterationRevision);
+        var previousIterationRevisionIndex = trac.indexOfRevision(previousIterationRevision);
+        if (previousIterationRevisionIndex === -1)
+            return content;
+
+        var firstRevision = trac.nextRevision(branch.name, previousIterationRevision);
+        if (firstRevision === Trac.NO_MORE_REVISIONS)
+            return content;
+        var firstRevisionIndex = trac.indexOfRevision(firstRevision);
+        console.assert(firstRevisionIndex !== -1);
+
+        var lastRevision = iteration.revision[repositoryName];
+        var lastRevisionIndex = trac.indexOfRevision(lastRevision);
+        if (lastRevisionIndex === -1)
+            return content;
+
+        if (firstRevisionIndex > lastRevisionIndex)
+            return content;
+
+        var context = {
+            trac: trac,
+            branch: branch,
+            firstRevision: firstRevision,
+            lastRevision: lastRevision,
+        };
+        new PopoverTracker(content, this._presentPopoverForRevisionRange.bind(this), context);
 
         return content;
     },
index a488d1c..546bb3d 100644 (file)
@@ -37,6 +37,8 @@ Trac = function(baseURL, options)
 
 BaseObject.addConstructorFunctions(Trac);
 
+Trac.NO_MORE_REVISIONS = null;
+
 Trac.NeedsAuthentication = "needsAuthentication";
 Trac.UpdateInterval = 45000; // 45 seconds
 
@@ -63,11 +65,40 @@ Trac.prototype = {
         return this.recordedCommits[this.recordedCommits.length - 1].revisionNumber;
     },
 
-    commitsOnBranch: function(branchName, filter)
+    _commitsOnBranch: function(branchName, beginPosition, endPosition)
+    {
+        beginPosition = beginPosition || 0;
+        if (endPosition === undefined)
+            endPosition = this.recordedCommits.length - 1;
+        var commits = [];
+        for (var i = beginPosition; i <= endPosition; ++i) {
+            var commit = this.recordedCommits[i];
+            if (!commit.containsBranchLocation || commit.branches.includes(branchName))
+                commits.push(commit);
+        }
+        return commits;
+    },
+
+    commitsOnBranchLaterThanRevision: function(branchName, revision)
+    {
+        var indexToBeLaterThan = this.indexOfRevision(revision);
+        console.assert(indexToBeLaterThan !== -1, revision + " is not in the list of recorded commits");
+        if (indexToBeLaterThan === -1)
+            return [];
+        return this._commitsOnBranch(branchName, indexToBeLaterThan + 1);
+    },
+
+    commitsOnBranchInRevisionRange: function(branchName, firstRevision, lastRevision)
     {
-        return this.recordedCommits.filter(function(commit, index, array) {
-            return (!commit.containsBranchLocation || commit.branches.includes(branchName)) && filter(commit, index, array);
-        });
+        var indexOfFirstRevision = this.indexOfRevision(firstRevision);
+        console.assert(indexOfFirstRevision !== -1, firstRevision + " is not in the list of recorded commits");
+        if (indexOfFirstRevision === -1)
+            return [];
+        var indexOfLastRevision = this.indexOfRevision(lastRevision);
+        console.assert(indexOfLastRevision !== -1, lastRevision + " is not in the list of recorded commits");
+        if (indexOfLastRevision === -1)
+            return [];
+        return this._commitsOnBranch(branchName, indexOfFirstRevision, indexOfLastRevision);
     },
 
     revisionURL: function(revision)
@@ -266,4 +297,22 @@ Trac.prototype = {
             this._loaded(dataDocument);
         }.bind(this), this._needsAuthentication ? { withCredentials: true } : {});
     },
+
+    nextRevision: function(branchName, revision)
+    {
+        var commits = this.commitsOnBranchLaterThanRevision(branchName, revision);
+        if (commits.length > 0)
+            return commits[0].revisionNumber;
+        return Trac.NO_MORE_REVISIONS;
+    },
+
+    indexOfRevision: function(revision)
+    {
+        var commits = this.recordedCommits;
+        for (var i = 0; i < commits.length; ++i) {
+            if (commits[i].revisionNumber === revision)
+                return i;
+        }
+        return -1;
+    },
 };
index 366f00b..969fab2 100644 (file)
@@ -72,6 +72,40 @@ test("_parseRevisionFromURL", function()
     strictEqual(this.trac._parseRevisionFromURL("https://git.foobar.com/trac/Whatever.git/changeset/0e498db5d8e5b5a342631"), "0e498db5d8e5b5a342631", "Git");
 });
 
+test("nextRevision", function()
+{
+    this.trac.recordedCommits = MockTrac.EXAMPLE_TRAC_COMMITS;
+    strictEqual(this.trac.nextRevision("trunk", 33020), 33022, "nextRevision same branch");
+    strictEqual(this.trac.nextRevision("trunk", 33019), 33020, "nextRevision different branch");
+});
+
+test("indexOfRevision", function()
+{
+    this.trac.recordedCommits = MockTrac.EXAMPLE_TRAC_COMMITS;
+    strictEqual(this.trac.indexOfRevision(33020), 2, "indexOfRevision");
+});
+
+test("commitsOnBranchLaterThanRevision", function()
+{
+    this.trac.recordedCommits = MockTrac.EXAMPLE_TRAC_COMMITS;
+    var commits = this.trac.commitsOnBranchLaterThanRevision("trunk", 33020);
+    equal(commits.length, 1, "greater than 33020");
+});
+
+test("commitsOnBranchLaterThanRevision no commits", function()
+{
+    this.trac.recordedCommits = MockTrac.EXAMPLE_TRAC_COMMITS;
+    var commits = this.trac.commitsOnBranchLaterThanRevision("someOtherBranch", 33021);
+    equal(commits.length, 0, "greater than 33021");
+});
+
+test("commitsOnBranchInRevisionRange", function()
+{
+    this.trac.recordedCommits = MockTrac.EXAMPLE_TRAC_COMMITS;
+    var commits = this.trac.commitsOnBranchInRevisionRange("trunk", 33020, 33022);
+    equal(commits.length, 2, "in range 33020, 33022");
+});
+
 module("BuildBotQueueView", {
     setup: function() {
         this.trac = new MockTrac();
@@ -113,6 +147,31 @@ test("_presentPopoverForPendingCommits", function()
     strictEqual(nodeList.length, 1, "has 1 pending commit");
 });
 
+test("_presentPopoverForPendingCommits no pending commits", function()
+{
+    this.someOtherBranch = {
+        name: "someOtherBranch",
+        repository: {
+            name: "openSource",
+            trac: this.trac,
+            isSVN: true,
+        }
+    };
+    this.queue.branches = [this.someOtherBranch];
+    this.view._latestProductiveIteration = function(queue)
+    {
+        var iteration = {
+            revision: { "openSource": 33021 },
+        };
+        return iteration;
+    }
+    var element = document.createElement("div");
+    var popover = new Dashboard.Popover();
+    this.view._presentPopoverForPendingCommits(element, popover, this.queue);
+    var nodeList = popover._element.getElementsByClassName("pending-commit");
+    strictEqual(nodeList.length, 0, "has 0 pending commits");
+});
+
 test("_presentPopoverForRevisionRange", function()
 {
     var element = document.createElement("div");
@@ -191,12 +250,13 @@ test("_formatRevisionForDisplay Git", function()
 
 module("BuildBotQueue", {
     setup: function() {
+        Dashboard.Repository.OpenSource.trac = new MockTrac();
+        Dashboard.Repository.OpenSource.trac.recordedCommits = MockTrac.EXAMPLE_TRAC_COMMITS;
+        Dashboard.Repository.Internal.trac = new MockTrac();
         this.queue = new MockBuildbotQueue();
         this.queue.branches = [{
             name: "trunk",
-            repository: {
-                name: "openSource",
-            }
+            repository: Dashboard.Repository.Opensource
         }];
     }
 });
index f62b7fb..54ef05a 100644 (file)
@@ -1,3 +1,29 @@
+2016-02-10  Jason Marcell  <jmarcell@apple.com>
+
+        Teach dashboard code to compare non-integer revisions
+        https://bugs.webkit.org/show_bug.cgi?id=152345
+
+        Reviewed by Daniel Bates.
+
+        * BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotQueue.js:
+        (BuildbotQueue.prototype.compareIterationsByRevisions): Compare non-integer revisions.
+        * BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotQueueView.js:
+        (BuildbotQueueView.prototype._appendPendingRevisionCount): Use Trac.indexOfRevision in order to compare non-integer
+        revisions. Also uses new Trac.commitsOnBranchLaterThanRevision method.
+        (BuildbotQueueView.prototype._popoverLinesForCommitRange): Ditto.
+        (BuildbotQueueView.prototype._presentPopoverForPendingCommits): Use Trac.indexOfRevision in order to compare non-integer
+        revisions. Also uses new Trac.nextRevision method to calculate a revision range.
+        (BuildbotQueueView.prototype._revisionContentWithPopoverForIteration): Ditto.
+        * BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/Trac.js:
+        (Trac.prototype._commitsOnBranch): Renamed this to indicate that it should be a private method used by the latter two new
+        methods.
+        (Trac.prototype.commitsOnBranchLaterThanRevision): Finds revisions on a branch later than the specified revision.
+        (Trac.prototype.commitsOnBranchInRevisionRange): Finds revisions on a branch within a specified range.
+        (Trac.prototype.nextRevision): Finds the next revision after a given revision on a specific branch.
+        (Trac.prototype.indexOfRevision): Finds the index of a given revision within the recordedCommits array.
+        (Trac.prototype.commitsOnBranch): Deleted. Renamed to _commitsOnBranch.
+        * BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/tests/tests.js: Added unit tests.
+
 2016-02-09  Nan Wang  <n_wang@apple.com>
 
         AX: Implement word related text marker functions using TextIterator