Wyatt Allen 206966a5ae Render inline diffs sequentially
Changes with large numbers of files could overwhelm PolyGerrit when a
user selects [Expand all] for inline diffs. This was because the
asynchronous processing/rendering/annotating process would be kicked off
for all unexpanded diffs simultaneously, resulting in browser lock-up
and general slowness even after rendering had completed.

With this change, inline diffs are rendered in serial rather than
parallel. In this way the benefits of the async features of diff
rendering extend to the file list, even for changes with many large
diffs (such as the one in the linked issue).

With this change, the `__expanded` property is removed from file objects
in GR-FILE-LIST. Instead, that element maintains a list
(`_expandedFilePaths`) which records the same information. Because the
expanded files are recorded in a list, however, splices on the list can
be observed, batch diff expansion can be handled sequentially.

Tests are updated to respect the new expanded paths list.

Bug: Issue 5396
Change-Id: Ib83ff5157177e1c890db8a82fbc25df8fecbe065
2017-02-07 16:17:56 -08:00

579 lines
18 KiB
HTML
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

<!DOCTYPE html>
<!--
Copyright (C) 2015 The Android Open Source Project
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
-->
<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
<title>gr-diff</title>
<script src="../../../bower_components/webcomponentsjs/webcomponents.min.js"></script>
<script src="../../../bower_components/web-component-tester/browser.js"></script>
<script src="../../../scripts/util.js"></script>
<link rel="import" href="../../../bower_components/iron-test-helpers/iron-test-helpers.html">
<link rel="import" href="gr-diff.html">
<test-fixture id="basic">
<template>
<gr-diff></gr-diff>
</template>
</test-fixture>
<script>
suite('gr-diff tests', function() {
var element;
var sandbox;
setup(function() {
sandbox = sinon.sandbox.create();
});
teardown(function() {
sandbox.restore();
});
suite('not logged in', function() {
setup(function() {
stub('gr-rest-api-interface', {
getLoggedIn: function() { return Promise.resolve(false); },
});
element = fixture('basic');
});
test('toggleLeftDiff', function() {
element.toggleLeftDiff();
assert.isTrue(element.classList.contains('no-left'));
element.toggleLeftDiff();
assert.isFalse(element.classList.contains('no-left'));
});
test('view does not start with displayLine classList', function() {
assert.isFalse(
element.$$('.diffContainer').classList.contains('displayLine'));
});
test('displayLine class added called when displayLine is true',
function() {
var spy = sandbox.spy(element, '_computeContainerClass');
element.displayLine = true;
assert.isTrue(spy.called);
assert.isTrue(
element.$$('.diffContainer').classList.contains('displayLine'));
});
test('get drafts', function(done) {
element.patchRange = {basePatchNum: 0, patchNum: 0};
var getDraftsStub = sandbox.stub(element.$.restAPI, 'getDiffDrafts');
element._getDiffDrafts().then(function(result) {
assert.deepEqual(result, {baseComments: [], comments: []});
sinon.assert.notCalled(getDraftsStub);
done();
});
});
test('get robot comments', function(done) {
element.patchRange = {basePatchNum: 0, patchNum: 0};
var getDraftsStub = sandbox.stub(element.$.restAPI,
'getDiffRobotComments');
element._getDiffDrafts().then(function(result) {
assert.deepEqual(result, {baseComments: [], comments: []});
sinon.assert.notCalled(getDraftsStub);
done();
});
});
test('loads files weblinks', function(done) {
var diffStub = sandbox.stub(element.$.restAPI, 'getDiff').returns(
Promise.resolve({
meta_a: {
web_links: 'foo',
},
meta_b: {
web_links: 'bar',
},
}));
element.patchRange = {};
element._getDiff().then(function() {
assert.deepEqual(element.filesWeblinks, {
meta_a: 'foo',
meta_b: 'bar',
});
done();
});
});
test('remove comment', function() {
element._comments = {
meta: {
changeNum: '42',
patchRange: {
basePatchNum: 'PARENT',
patchNum: 3,
},
path: '/path/to/foo',
projectConfig: {foo: 'bar'},
},
left: [
{id: 'bc1', side: 'PARENT'},
{id: 'bc2', side: 'PARENT'},
{id: 'bd1', __draft: true, side: 'PARENT'},
{id: 'bd2', __draft: true, side: 'PARENT'},
],
right: [
{id: 'c1'},
{id: 'c2'},
{id: 'd1', __draft: true},
{id: 'd2', __draft: true},
],
};
element._removeComment({});
// Using JSON.stringify because Safari 9.1 (11601.5.17.1) doesnt seem
// to believe that one object deepEquals another even when they do :-/.
assert.equal(JSON.stringify(element._comments), JSON.stringify({
meta: {
changeNum: '42',
patchRange: {
basePatchNum: 'PARENT',
patchNum: 3,
},
path: '/path/to/foo',
projectConfig: {foo: 'bar'},
},
left: [
{id: 'bc1', side: 'PARENT'},
{id: 'bc2', side: 'PARENT'},
{id: 'bd1', __draft: true, side: 'PARENT'},
{id: 'bd2', __draft: true, side: 'PARENT'},
],
right: [
{id: 'c1'},
{id: 'c2'},
{id: 'd1', __draft: true},
{id: 'd2', __draft: true},
],
}));
element._removeComment({id: 'bc2', side: 'PARENT'});
assert.equal(JSON.stringify(element._comments), JSON.stringify({
meta: {
changeNum: '42',
patchRange: {
basePatchNum: 'PARENT',
patchNum: 3,
},
path: '/path/to/foo',
projectConfig: {foo: 'bar'},
},
left: [
{id: 'bc1', side: 'PARENT'},
{id: 'bd1', __draft: true, side: 'PARENT'},
{id: 'bd2', __draft: true, side: 'PARENT'},
],
right: [
{id: 'c1'},
{id: 'c2'},
{id: 'd1', __draft: true},
{id: 'd2', __draft: true},
],
}));
element._removeComment({id: 'd2'});
assert.deepEqual(JSON.stringify(element._comments), JSON.stringify({
meta: {
changeNum: '42',
patchRange: {
basePatchNum: 'PARENT',
patchNum: 3,
},
path: '/path/to/foo',
projectConfig: {foo: 'bar'},
},
left: [
{id: 'bc1', side: 'PARENT'},
{id: 'bd1', __draft: true, side: 'PARENT'},
{id: 'bd2', __draft: true, side: 'PARENT'},
],
right: [
{id: 'c1'},
{id: 'c2'},
{id: 'd1', __draft: true},
],
}));
});
test('thread groups', function() {
var contentEl = document.createElement('div');
var rangeToCheck = 'line';
var patchNum = 1;
var side = 'PARENT';
var range = {
startLine: 1,
startChar: 1,
endLine: 1,
endChar: 2,
};
sandbox.stub(element.$.diffBuilder, 'createCommentThreadGroup',
function() {
return document.createElement('gr-diff-comment-thread-group');
});
// No thread groups.
assert.isNotOk(element._getThreadGroupForLine(contentEl));
// A thread group gets created.
assert.isOk(
element._getOrCreateThreadAtLineRange(contentEl, patchNum, side));
// Try to fetch a thread with a different range.
range = {
startLine: 1,
startChar: 1,
endLine: 1,
endChar: 3,
};
assert.isOk(element._getOrCreateThreadAtLineRange(
contentEl, patchNum, side, range));
// The new thread group can be fetched.
assert.isOk(element._getThreadGroupForLine(contentEl));
assert.equal(contentEl.querySelectorAll(
'gr-diff-comment-thread-group').length, 1);
var threadGroup = contentEl.querySelector(
'gr-diff-comment-thread-group');
var threadLength = Polymer.dom(threadGroup.root).
querySelectorAll('gr-diff-comment-thread').length;
assert.equal(threadLength, 2);
});
test('renders image diffs', function(done) {
var mockDiff = {
meta_a: {name: 'carrot.jpg', content_type: 'image/jpeg', lines: 66},
meta_b: {name: 'carrot.jpg', content_type: 'image/jpeg', lines: 560},
intraline_status: 'OK',
change_type: 'MODIFIED',
diff_header: [
'diff --git a/carrot.jpg b/carrot.jpg',
'index 2adc47d..f9c2f2c 100644',
'--- a/carrot.jpg',
'+++ b/carrot.jpg',
'Binary files differ',
],
content: [{skip: 66}],
binary: true,
};
var mockFile1 = {
body: 'Qk06AAAAAAAAADYAAAAoAAAAAQAAAP////8BACAAAAAAAAAAAAATCwAAEwsA' +
'AAAAAAAAAAAAAAAA/w==',
type: 'image/bmp',
};
var mockFile2 = {
body: 'Qk06AAAAAAAAADYAAAAoAAAAAQAAAP////8BACAAAAAAAAAAAAATCwAAEwsA' +
'AAAAAAAAAAAA/////w==',
type: 'image/bmp'
};
var mockCommit = {
commit: '9a1a1d10baece5efbba10bc4ccf808a67a50ac0a',
parents: [{
commit: '7338aa9adfe57909f1fdaf88975cdea467d3382f',
subject: 'Added a carrot',
}],
author: {
name: 'Wyatt Allen',
email: 'wyatta@google.com',
date: '2016-05-23 21:44:51.000000000',
tz: -420,
},
committer: {
name: 'Wyatt Allen',
email: 'wyatta@google.com',
date: '2016-05-25 00:25:41.000000000',
tz: -420,
},
subject: 'Updated the carrot',
message: 'Updated the carrot\n\nChange-Id: Iabcd123\n',
};
var mockComments = {baseComments: [], comments: []};
var stubs = [];
stubs.push(sandbox.stub(element, '_getDiff',
function() { return Promise.resolve(mockDiff); }));
stubs.push(sandbox.stub(element.$.restAPI, 'getCommitInfo',
function() { return Promise.resolve(mockCommit); }));
stubs.push(sandbox.stub(element.$.restAPI,
'getCommitFileContents',
function() { return Promise.resolve(mockFile1); }));
stubs.push(sandbox.stub(element.$.restAPI,
'getChangeFileContents',
function() { return Promise.resolve(mockFile2); }));
stubs.push(sandbox.stub(element.$.restAPI, '_getDiffComments',
function() { return Promise.resolve(mockComments); }));
stubs.push(sandbox.stub(element.$.restAPI, 'getDiffDrafts',
function() { return Promise.resolve(mockComments); }));
element.patchRange = {basePatchNum: 'PARENT', patchNum: 1};
var rendered = function() {
// Recognizes that it should be an image diff.
assert.isTrue(element.isImageDiff);
assert.instanceOf(element.$.diffBuilder._builder, GrDiffBuilderImage);
// Left image rendered with the parent commit's version of the file.
var leftInmage = element.$.diffTable.querySelector('td.left img');
assert.isOk(leftInmage);
assert.equal(leftInmage.getAttribute('src'),
'data:image/bmp;base64, ' + mockFile1.body);
// Right image rendered with this change's revision of the image.
var rightInmage = element.$.diffTable.querySelector('td.right img');
assert.isOk(rightInmage);
assert.equal(rightInmage.getAttribute('src'),
'data:image/bmp;base64, ' + mockFile2.body);
// Cleanup.
element.removeEventListener('render', rendered);
done();
};
element.addEventListener('render', rendered);
element.$.restAPI.getDiffPreferences().then(function(prefs) {
element.prefs = prefs;
element.reload();
});
});
test('_handleTap lineNum', function(done) {
var addDraftStub = sinon.stub(element, 'addDraftAtLine');
var el = document.createElement('div');
el.className = 'lineNum';
el.addEventListener('click', function(e) {
element._handleTap(e);
assert.isTrue(addDraftStub.called);
assert.equal(addDraftStub.lastCall.args[0], el);
done();
});
el.click();
});
test('_handleTap context', function(done) {
var showContextStub = sinon.stub(element.$.diffBuilder, 'showContext');
var el = document.createElement('div');
el.className = 'showContext';
el.addEventListener('click', function(e) {
element._handleTap(e);
assert.isTrue(showContextStub.called);
done();
});
el.click();
});
test('_handleTap content', function(done) {
var content = document.createElement('div');
var lineEl = document.createElement('div');
var selectStub = sandbox.stub(element, '_selectLine');
var getLineStub = sandbox.stub(element.$.diffBuilder,
'getLineElByChild', function() { return lineEl; });
content.className = 'content';
content.addEventListener('click', function(e) {
element._handleTap(e);
assert.isTrue(selectStub.called);
assert.equal(selectStub.lastCall.args[0], lineEl);
done();
});
content.click();
});
test('_getDiff handles null diff responses', function(done) {
stub('gr-rest-api-interface', {
getDiff: function() { return Promise.resolve(null); },
});
element.changeNum = 123;
element.patchRange = {basePatchNum: 1, patchNum: 2};
element.path = 'file.txt';
element._getDiff().then(done);
});
});
suite('logged in', function() {
setup(function() {
stub('gr-rest-api-interface', {
getLoggedIn: function() { return Promise.resolve(true); },
getPreferences: function() {
return Promise.resolve({time_format: 'HHMM_12'});
},
});
element = fixture('basic');
});
test('get drafts', function(done) {
element.patchRange = {basePatchNum: 0, patchNum: 0};
var draftsResponse = {
baseComments: [{id: 'foo'}],
comments: [{id: 'bar'}],
};
var getDraftsStub = sandbox.stub(element.$.restAPI, 'getDiffDrafts',
function() { return Promise.resolve(draftsResponse); });
element._getDiffDrafts().then(function(result) {
assert.deepEqual(result, draftsResponse);
done();
});
});
test('get comments and drafts', function(done) {
var comments = {
baseComments: [
{id: 'bc1'},
{id: 'bc2'},
],
comments: [
{id: 'c1'},
{id: 'c2'},
],
};
var diffCommentsStub = sandbox.stub(element, '_getDiffComments',
function() { return Promise.resolve(comments); });
var drafts = {
baseComments: [
{id: 'bd1'},
{id: 'bd2'},
],
comments: [
{id: 'd1'},
{id: 'd2'},
],
};
var diffDraftsStub = sandbox.stub(element, '_getDiffDrafts',
function() { return Promise.resolve(drafts); });
var robotComments = {
baseComments: [
{id: 'br1'},
{id: 'br2'},
],
comments: [
{id: 'r1'},
{id: 'r2'},
],
};
var diffRobotCommentStub = sandbox.stub(element,
'_getDiffRobotComments', function() {
return Promise.resolve(robotComments); });
element.changeNum = '42';
element.patchRange = {
basePatchNum: 'PARENT',
patchNum: 3,
};
element.path = '/path/to/foo';
element.projectConfig = {foo: 'bar'};
element._getDiffCommentsAndDrafts().then(function(result) {
assert.deepEqual(result, {
meta: {
changeNum: '42',
patchRange: {
basePatchNum: 'PARENT',
patchNum: 3,
},
path: '/path/to/foo',
projectConfig: {foo: 'bar'},
},
left: [
{id: 'bc1'},
{id: 'bc2'},
{id: 'bd1', __draft: true},
{id: 'bd2', __draft: true},
{id: 'br1'},
{id: 'br2'},
],
right: [
{id: 'c1'},
{id: 'c2'},
{id: 'd1', __draft: true},
{id: 'd2', __draft: true},
{id: 'r1'},
{id: 'r2'},
],
});
done();
});
});
suite('handle comment-update', function() {
setup(function() {
element._comments = {
meta: {
changeNum: '42',
patchRange: {
basePatchNum: 'PARENT',
patchNum: 3,
},
path: '/path/to/foo',
projectConfig: {foo: 'bar'},
},
left: [
{id: 'bc1', side: 'PARENT'},
{id: 'bc2', side: 'PARENT'},
{id: 'bd1', __draft: true, side: 'PARENT'},
{id: 'bd2', __draft: true, side: 'PARENT'},
],
right: [
{id: 'c1'},
{id: 'c2'},
{id: 'd1', __draft: true},
{id: 'd2', __draft: true},
],
};
});
test('creating a draft', function() {
var comment = {__draft: true, __draftID: 'tempID', side: 'PARENT'};
element.fire('comment-update', {comment: comment});
assert.include(element._comments.left, comment);
});
test('saving a draft', function() {
var draftID = 'tempID';
var id = 'savedID';
element._comments.left.push(
{__draft: true, __draftID: draftID, side: 'PARENT'});
element.fire('comment-update', {comment:
{id: id, __draft: true, __draftID: draftID, side: 'PARENT'},
});
var drafts = element._comments.left.filter(function(item) {
return item.__draftID === draftID;
});
assert.equal(drafts.length, 1);
assert.equal(drafts[0].id, id);
});
});
});
});
</script>