opendev/gerrit
Code Issues Proposed changes

Move all server calls to gr-diff-host

Browse Source 
This way gr-diff can be reused with a different server.

I decided to move tests concerning the server calls almost unchanged to
gr-diff-host, even when they reach into gr-diff and deeper to check if
certain things happened as a result of loading.

I added similar tests, minus the actual loading, back to gr-diff. This
results in a bit of duplicated test logic, because e.g. the rendering
is tested both in gr-diff and gr-diff-host. I think that is worth it to
reduce the likelihood I broke something.

Bug: Issue 9623
Change-Id: Ib460094eb8d5b8e856c0d7954ee801fa3b5946d3
This commit is contained in:
Ole Rehmsen
2018-08-27 22:43:38 +02:00
parent ffb320ae65
commit b1a7967be2
8 changed files with 963 additions and 733 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
View File

@@ -1351,7 +1351,6 @@ limitations under the License.
const setupDiff = function(diff) {
const mock = document.createElement('mock-diff-response');
diff.$.diff._diff = mock.diffResponse;
diff.comments = {
left: diff.path === '/COMMIT_MSG' ? commitMsgComments : [],
right: [],
@@ -1379,7 +1378,7 @@ limitations under the License.
theme: 'DEFAULT',
ignore_whitespace: 'IGNORE_NONE',
};
diff.$.diff._renderDiffTable();
diff._diff = mock.diffResponse;
};
const renderAndGetNewDiffs = function(index) {

23
polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html
View File

@@ -35,6 +35,7 @@ limitations under the License.
<mock-diff-response></mock-diff-response>
<gr-diff></gr-diff>
<gr-diff-cursor></gr-diff-cursor>
<gr-rest-api-interface></gr-rest-api-interface>
</template>
</test-fixture>
@@ -48,27 +49,18 @@ limitations under the License.
setup(done => {
sandbox = sinon.sandbox.create();
stub('gr-rest-api-interface', {
getLoggedIn() { return Promise.resolve(false); },
getDiff() {
return Promise.resolve(mockDiffResponse.diffResponse);
},
});
const fixtureElems = fixture('basic');
mockDiffResponse = fixtureElems[0];
diffElement = fixtureElems[1];
cursorElement = fixtureElems[2];
const restAPI = fixtureElems[3];
// Register the diff with the cursor.
cursorElement.push('diffs', diffElement);
diffElement.loggedIn = false;
diffElement.patchRange = {basePatchNum: 1, patchNum: 2};
diffElement.comments = {left: [], right: []};
diffElement.$.restAPI.getDiffPreferences().then(prefs => {
diffElement.prefs = prefs;
});
const setupDone = () => {
cursorElement._updateStops();
cursorElement.moveToFirstChunk();
@@ -77,7 +69,10 @@ limitations under the License.
};
diffElement.addEventListener('render', setupDone);
diffElement.reload();
restAPI.getDiffPreferences().then(prefs => {
diffElement.prefs = prefs;
diffElement.diff = mockDiffResponse.diffResponse;
});
});
teardown(() => sandbox.restore());
@@ -219,7 +214,7 @@ limitations under the License.
done();
}
diffElement.addEventListener('render', renderHandler);
diffElement.reload();
diffElement._diffChanged(mockDiffResponse.diffResponse);
});
test('initialLineNumber enabled', done => {
@@ -239,7 +234,7 @@ limitations under the License.
cursorElement.initialLineNumber = 10;
cursorElement.side = 'right';
diffElement.reload();
diffElement._diffChanged(mockDiffResponse.diffResponse);
});
test('getAddress', () => {

17
polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html
View File

@@ -16,6 +16,9 @@ limitations under the License.
-->
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../core/gr-reporting/gr-reporting.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
<link rel="import" href="../gr-diff/gr-diff.html">
<dom-module id="gr-diff-host">
@@ -30,17 +33,23 @@ limitations under the License.
project-config="[[projectConfig]]"
project-name="[[projectName]]"
display-line="[[displayLine]]"
is-image-diff="{{isImageDiff}}"
is-image-diff="[[isImageDiff]]"
commit-range="[[commitRange]]"
files-weblinks="{{filesWeblinks}}"
hidden$="[[hidden]]"
no-render-on-prefs-change="[[noRenderOnPrefsChange]]"
comments="[[comments]]"
line-wrapping="[[lineWrapping]]"
view-mode="[[viewMode]]"
line-of-interest="[[lineOfInterest]]"
show-load-failure="[[showLoadFailure]]"
is-blame-loaded="{{isBlameLoaded}}"></gr-diff>
logged-in="[[_loggedIn]]"
loading="[[_loading]]"
error-message="[[_errorMessage]]"
base-image="[[_baseImage]]"
revision-image=[[_revisionImage]]
blame="[[_blame]]"
diff="[[_diff]]"></gr-diff>
<gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
<gr-reporting id="reporting" category="diff"></gr-reporting>
</template>
<script src="gr-diff-host.js"></script>
</dom-module>

266
polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
View File

@@ -17,11 +17,32 @@
(function() {
'use strict';
const MSG_EMPTY_BLAME = 'No blame information for this diff.';
const EVENT_AGAINST_PARENT = 'diff-against-parent';
const EVENT_ZERO_REBASE = 'rebase-percent-zero';
const EVENT_NONZERO_REBASE = 'rebase-percent-nonzero';
const DiffViewMode = {
SIDE_BY_SIDE: 'SIDE_BY_SIDE',
UNIFIED: 'UNIFIED_DIFF',
};
/**
* @param {Object} diff
* @return {boolean}
*/
function isImageDiff(diff) {
if (!diff) { return false; }
const isA = diff.meta_a &&
diff.meta_a.content_type.startsWith('image/');
const isB = diff.meta_b &&
diff.meta_b.content_type.startsWith('image/');
return !!(diff.binary && (isA || isB));
}
/**
* Wrapper around gr-diff.
*
@@ -71,11 +92,13 @@
},
isImageDiff: {
type: Boolean,
computed: '_computeIsImageDiff(_diff)',
notify: true,
},
commitRange: Object,
filesWeblinks: {
type: Object,
value() { return {}; },
notify: true,
},
hidden: {
@@ -113,12 +136,109 @@
isBlameLoaded: {
type: Boolean,
notify: true,
computed: '_computeIsBlameLoaded(_blame)',
},
_loggedIn: {
type: Boolean,
value: false,
},
_loading: {
type: Boolean,
value: false,
},
/** @type {?string} */
_errorMessage: {
type: String,
value: null,
},
/** @type {?Object} */
_baseImage: Object,
/** @type {?Object} */
_revisionImage: Object,
_diff: Object,
/** @type {?Object} */
_blame: {
type: Object,
value: null,
},
},
listeners: {
'draft-interaction': '_handleDraftInteraction',
},
ready() {
if (this._canReload()) {
this.reload();
}
},
attached() {
this._getLoggedIn().then(loggedIn => {
this._loggedIn = loggedIn;
});
},
/** @return {!Promise} */
reload() {
return this.$.diff.reload();
this._loading = true;
this._errorMessage = null;
const diffRequest = this._getDiff()
.then(diff => {
this._reportDiff(diff);
return diff;
})
.catch(e => {
this._handleGetDiffError(e);
return null;
});
const assetRequest = diffRequest.then(diff => {
// If the diff is null, then it's failed to load.
if (!diff) { return null; }
return this._loadDiffAssets(diff);
});
return Promise.all([diffRequest, assetRequest])
.then(results => {
const diff = results[0];
if (!diff) {
return Promise.resolve();
}
this.filesWeblinks = this._getFilesWeblinks(diff);
return new Promise(resolve => {
const callback = () => {
resolve();
this.removeEventListener('render', callback);
};
this.addEventListener('render', callback);
this._diff = diff;
});
})
.catch(err => {
console.warn('Error encountered loading diff:', err);
})
.then(() => { this._loading = false; });
},
_getFilesWeblinks(diff) {
if (!this.commitRange) { return {}; }
return {
meta_a: Gerrit.Nav.getFileWebLinks(
this.projectName, this.commitRange.baseCommit, this.path,
{weblinks: diff && diff.meta_a && diff.meta_a.web_links}),
meta_b: Gerrit.Nav.getFileWebLinks(
this.projectName, this.commitRange.commit, this.path,
{weblinks: diff && diff.meta_b && diff.meta_b.web_links}),
};
},
/** Cancel any remaining diff builder rendering work. */
@@ -145,12 +265,21 @@
* @return {Promise} A promise that resolves when blame finishes rendering.
*/
loadBlame() {
return this.$.diff.loadBlame();
return this.$.restAPI.getBlame(this.changeNum, this.patchRange.patchNum,
this.path, true)
.then(blame => {
if (!blame.length) {
this.fire('show-alert', {message: MSG_EMPTY_BLAME});
return Promise.reject(MSG_EMPTY_BLAME);
}
this._blame = blame;
});
},
/** Unload blame information for the diff. */
clearBlame() {
this.$.diff.clearBlame();
this._blame = null;
},
/** @return {!Array<!HTMLElement>} */
@@ -170,5 +299,136 @@
expandAllContext() {
this.$.diff.expandAllContext();
},
/** @return {!Promise} */
_getLoggedIn() {
return this.$.restAPI.getLoggedIn();
},
/** @return {boolean}} */
_canReload() {
return !!this.changeNum && !!this.patchRange && !!this.path &&
!this.noAutoRender;
},
/** @return {!Promise<!Object>} */
_getDiff() {
// Wrap the diff request in a new promise so that the error handler
// rejects the promise, allowing the error to be handled in the .catch.
return new Promise((resolve, reject) => {
this.$.restAPI.getDiff(
this.changeNum,
this.patchRange.basePatchNum,
this.patchRange.patchNum,
this.path,
reject)
.then(resolve);
});
},
_handleGetDiffError(response) {
// Loading the diff may respond with 409 if the file is too large. In this
// case, use a toast error..
if (response.status === 409) {
this.fire('server-error', {response});
return;
}
if (this.showLoadFailure) {
this._errorMessage = [
'Encountered error when loading the diff:',
response.status,
response.statusText,
].join(' ');
return;
}
this.fire('page-error', {response});
},
/**
* Report info about the diff response.
*/
_reportDiff(diff) {
if (!diff || !diff.content) { return; }
// Count the delta lines stemming from normal deltas, and from
// due_to_rebase deltas.
let nonRebaseDelta = 0;
let rebaseDelta = 0;
diff.content.forEach(chunk => {
if (chunk.ab) { return; }
const deltaSize = Math.max(
chunk.a ? chunk.a.length : 0, chunk.b ? chunk.b.length : 0);
if (chunk.due_to_rebase) {
rebaseDelta += deltaSize;
} else {
nonRebaseDelta += deltaSize;
}
});
// Find the percent of the delta from due_to_rebase chunks rounded to two
// digits. Diffs with no delta are considered 0%.
const totalDelta = rebaseDelta + nonRebaseDelta;
const percentRebaseDelta = !totalDelta ? 0 :
Math.round(100 * rebaseDelta / totalDelta);
// Report the due_to_rebase percentage in the "diff" category when
// applicable.
if (this.patchRange.basePatchNum === 'PARENT') {
this.$.reporting.reportInteraction(EVENT_AGAINST_PARENT);
} else if (percentRebaseDelta === 0) {
this.$.reporting.reportInteraction(EVENT_ZERO_REBASE);
} else {
this.$.reporting.reportInteraction(EVENT_NONZERO_REBASE,
percentRebaseDelta);
}
},
/**
* @param {Object} diff
* @return {!Promise}
*/
_loadDiffAssets(diff) {
if (isImageDiff(diff)) {
return this._getImages(diff).then(images => {
this._baseImage = images.baseImage;
this._revisionImage = images.revisionImage;
});
} else {
this._baseImage = null;
this._revisionImage = null;
return Promise.resolve();
}
},
/**
* @param {Object} diff
* @return {boolean}
*/
_computeIsImageDiff(diff) {
return isImageDiff(diff);
},
/**
* @param {Object} blame
* @return {boolean}
*/
_computeIsBlameLoaded(blame) {
return !!blame;
},
/**
* @param {Object} diff
* @return {!Promise}
*/
_getImages(diff) {
return this.$.restAPI.getImagesForDiff(this.changeNum, diff,
this.patchRange);
},
_handleDraftInteraction() {
this.$.reporting.recordDraftInteraction();
},
});
})();

643
polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
View File

@@ -40,23 +40,437 @@ limitations under the License.
setup(() => {
sandbox = sinon.sandbox.create();
element = fixture('basic');
});
teardown(() => {
sandbox.restore();
});
test('delegates reload()', () => {
element = fixture('basic');
const returnValue = Promise.resolve();
const stub = sandbox.stub(element.$.diff, 'reload').returns(returnValue);
assert.equal(element.reload(), returnValue);
assert.isTrue(stub.calledOnce);
assert.equal(stub.lastCall.args.length, 0);
test('reload() cancels before network resolves', () => {
const cancelStub = sandbox.stub(element.$.diff, 'cancel');
// Stub the network calls into requests that never resolve.
sandbox.stub(element, '_getDiff', () => new Promise(() => {}));
element.reload();
assert.isTrue(cancelStub.called);
});
suite('not logged in', () => {
setup(() => {
const getLoggedInPromise = Promise.resolve(false);
stub('gr-rest-api-interface', {
getLoggedIn() { return getLoggedInPromise; },
});
element = fixture('basic');
return getLoggedInPromise;
});
test('reload() loads files weblinks', () => {
const weblinksStub = sandbox.stub(Gerrit.Nav, '_generateWeblinks')
.returns({name: 'stubb', url: '#s'});
sandbox.stub(element.$.restAPI, 'getDiff').returns(Promise.resolve({
content: [],
}));
element.projectName = 'test-project';
element.path = 'test-path';
element.commitRange = {baseCommit: 'test-base', commit: 'test-commit'};
element.patchRange = {};
return element.reload().then(() => {
assert.isTrue(weblinksStub.calledTwice);
assert.isTrue(weblinksStub.firstCall.calledWith({
commit: 'test-base',
file: 'test-path',
options: {
weblinks: undefined,
},
repo: 'test-project',
type: Gerrit.Nav.WeblinkType.FILE}));
assert.isTrue(weblinksStub.secondCall.calledWith({
commit: 'test-commit',
file: 'test-path',
options: {
weblinks: undefined,
},
repo: 'test-project',
type: Gerrit.Nav.WeblinkType.FILE}));
assert.deepEqual(element.filesWeblinks, {
meta_a: [{name: 'stubb', url: '#s'}],
meta_b: [{name: 'stubb', url: '#s'}],
});
});
});
test('_getDiff handles null diff responses', done => {
stub('gr-rest-api-interface', {
getDiff() { return Promise.resolve(null); },
});
element.changeNum = 123;
element.patchRange = {basePatchNum: 1, patchNum: 2};
element.path = 'file.txt';
element._getDiff().then(done);
});
test('reload resolves on error', () => {
const onErrStub = sandbox.stub(element, '_handleGetDiffError');
const error = {ok: false, status: 500};
sandbox.stub(element.$.restAPI, 'getDiff',
(changeNum, basePatchNum, patchNum, path, onErr) => {
onErr(error);
});
return element.reload().then(() => {
assert.isTrue(onErrStub.calledOnce);
});
});
suite('_handleGetDiffError', () => {
let serverErrorStub;
let pageErrorStub;
setup(() => {
serverErrorStub = sinon.stub();
element.addEventListener('server-error', serverErrorStub);
pageErrorStub = sinon.stub();
element.addEventListener('page-error', pageErrorStub);
});
test('page error on HTTP-409', () => {
element._handleGetDiffError({status: 409});
assert.isTrue(serverErrorStub.calledOnce);
assert.isFalse(pageErrorStub.called);
assert.isNotOk(element._errorMessage);
});
test('server error on non-HTTP-409', () => {
element._handleGetDiffError({status: 500});
assert.isFalse(serverErrorStub.called);
assert.isTrue(pageErrorStub.calledOnce);
assert.isNotOk(element._errorMessage);
});
test('error message if showLoadFailure', () => {
element.showLoadFailure = true;
element._handleGetDiffError({status: 500, statusText: 'Failure!'});
assert.isFalse(serverErrorStub.called);
assert.isFalse(pageErrorStub.called);
assert.equal(element._errorMessage,
'Encountered error when loading the diff: 500 Failure!');
});
});
suite('image diffs', () => {
let mockFile1;
let mockFile2;
setup(() => {
mockFile1 = {
body: 'Qk06AAAAAAAAADYAAAAoAAAAAQAAAP////8BACAAAAAAAAAAAAATCwAAE' +
'wsAAAAAAAAAAAAAAAAA/w==',
type: 'image/bmp',
};
mockFile2 = {
body: 'Qk06AAAAAAAAADYAAAAoAAAAAQAAAP////8BACAAAAAAAAAAAAATCwAAE' +
'wsAAAAAAAAAAAAA/////w==',
type: 'image/bmp',
};
sandbox.stub(element.$.restAPI,
'getB64FileContents',
(changeId, patchNum, path, opt_parentIndex) => {
return Promise.resolve(opt_parentIndex === 1 ? mockFile1 :
mockFile2);
});
element.patchRange = {basePatchNum: 'PARENT', patchNum: 1};
element.comments = {left: [], right: []};
});
test('renders image diffs with same file name', done => {
const 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,
};
sandbox.stub(element.$.restAPI, 'getDiff')
.returns(Promise.resolve(mockDiff));
const rendered = () => {
// Recognizes that it should be an image diff.
assert.isTrue(element.isImageDiff);
assert.instanceOf(
element.$.diff.$.diffBuilder._builder, GrDiffBuilderImage);
// Left image rendered with the parent commit's version of the file.
const leftImage =
element.$.diff.$.diffTable.querySelector('td.left img');
const leftLabel =
element.$.diff.$.diffTable.querySelector('td.left label');
const leftLabelContent = leftLabel.querySelector('.label');
const leftLabelName = leftLabel.querySelector('.name');
const rightImage =
element.$.diff.$.diffTable.querySelector('td.right img');
const rightLabel = element.$.diff.$.diffTable.querySelector(
'td.right label');
const rightLabelContent = rightLabel.querySelector('.label');
const rightLabelName = rightLabel.querySelector('.name');
assert.isNotOk(rightLabelName);
assert.isNotOk(leftLabelName);
let leftLoaded = false;
let rightLoaded = false;
leftImage.addEventListener('load', () => {
assert.isOk(leftImage);
assert.equal(leftImage.getAttribute('src'),
'data:image/bmp;base64, ' + mockFile1.body);
assert.equal(leftLabelContent.textContent, '1×1 image/bmp');
leftLoaded = true;
if (rightLoaded) {
element.removeEventListener('render', rendered);
done();
}
});
rightImage.addEventListener('load', () => {
assert.isOk(rightImage);
assert.equal(rightImage.getAttribute('src'),
'data:image/bmp;base64, ' + mockFile2.body);
assert.equal(rightLabelContent.textContent, '1×1 image/bmp');
rightLoaded = true;
if (leftLoaded) {
element.removeEventListener('render', rendered);
done();
}
});
};
element.addEventListener('render', rendered);
element.$.restAPI.getDiffPreferences().then(prefs => {
element.prefs = prefs;
element.reload();
});
});
test('renders image diffs with a different file name', done => {
const mockDiff = {
meta_a: {name: 'carrot.jpg', content_type: 'image/jpeg', lines: 66},
meta_b: {name: 'carrot2.jpg', content_type: 'image/jpeg',
lines: 560},
intraline_status: 'OK',
change_type: 'MODIFIED',
diff_header: [
'diff --git a/carrot.jpg b/carrot2.jpg',
'index 2adc47d..f9c2f2c 100644',
'--- a/carrot.jpg',
'+++ b/carrot2.jpg',
'Binary files differ',
],
content: [{skip: 66}],
binary: true,
};
sandbox.stub(element.$.restAPI, 'getDiff')
.returns(Promise.resolve(mockDiff));
const rendered = () => {
// Recognizes that it should be an image diff.
assert.isTrue(element.isImageDiff);
assert.instanceOf(
element.$.diff.$.diffBuilder._builder, GrDiffBuilderImage);
// Left image rendered with the parent commit's version of the file.
const leftImage =
element.$.diff.$.diffTable.querySelector('td.left img');
const leftLabel =
element.$.diff.$.diffTable.querySelector('td.left label');
const leftLabelContent = leftLabel.querySelector('.label');
const leftLabelName = leftLabel.querySelector('.name');
const rightImage =
element.$.diff.$.diffTable.querySelector('td.right img');
const rightLabel = element.$.diff.$.diffTable.querySelector(
'td.right label');
const rightLabelContent = rightLabel.querySelector('.label');
const rightLabelName = rightLabel.querySelector('.name');
assert.isOk(rightLabelName);
assert.isOk(leftLabelName);
assert.equal(leftLabelName.textContent, mockDiff.meta_a.name);
assert.equal(rightLabelName.textContent, mockDiff.meta_b.name);
let leftLoaded = false;
let rightLoaded = false;
leftImage.addEventListener('load', () => {
assert.isOk(leftImage);
assert.equal(leftImage.getAttribute('src'),
'data:image/bmp;base64, ' + mockFile1.body);
assert.equal(leftLabelContent.textContent, '1×1 image/bmp');
leftLoaded = true;
if (rightLoaded) {
element.removeEventListener('render', rendered);
done();
}
});
rightImage.addEventListener('load', () => {
assert.isOk(rightImage);
assert.equal(rightImage.getAttribute('src'),
'data:image/bmp;base64, ' + mockFile2.body);
assert.equal(rightLabelContent.textContent, '1×1 image/bmp');
rightLoaded = true;
if (leftLoaded) {
element.removeEventListener('render', rendered);
done();
}
});
};
element.addEventListener('render', rendered);
element.$.restAPI.getDiffPreferences().then(prefs => {
element.prefs = prefs;
element.reload();
});
});
test('renders added image', done => {
const mockDiff = {
meta_b: {name: 'carrot.jpg', content_type: 'image/jpeg',
lines: 560},
intraline_status: 'OK',
change_type: 'ADDED',
diff_header: [
'diff --git a/carrot.jpg b/carrot.jpg',
'index 0000000..f9c2f2c 100644',
'--- /dev/null',
'+++ b/carrot.jpg',
'Binary files differ',
],
content: [{skip: 66}],
binary: true,
};
sandbox.stub(element.$.restAPI, 'getDiff')
.returns(Promise.resolve(mockDiff));
element.addEventListener('render', () => {
// Recognizes that it should be an image diff.
assert.isTrue(element.isImageDiff);
assert.instanceOf(
element.$.diff.$.diffBuilder._builder, GrDiffBuilderImage);
const leftImage =
element.$.diff.$.diffTable.querySelector('td.left img');
const rightImage =
element.$.diff.$.diffTable.querySelector('td.right img');
assert.isNotOk(leftImage);
assert.isOk(rightImage);
done();
});
element.$.restAPI.getDiffPreferences().then(prefs => {
element.prefs = prefs;
element.reload();
});
});
test('renders removed image', done => {
const mockDiff = {
meta_a: {name: 'carrot.jpg', content_type: 'image/jpeg',
lines: 560},
intraline_status: 'OK',
change_type: 'DELETED',
diff_header: [
'diff --git a/carrot.jpg b/carrot.jpg',
'index f9c2f2c..0000000 100644',
'--- a/carrot.jpg',
'+++ /dev/null',
'Binary files differ',
],
content: [{skip: 66}],
binary: true,
};
sandbox.stub(element.$.restAPI, 'getDiff')
.returns(Promise.resolve(mockDiff));
element.addEventListener('render', () => {
// Recognizes that it should be an image diff.
assert.isTrue(element.isImageDiff);
assert.instanceOf(
element.$.diff.$.diffBuilder._builder, GrDiffBuilderImage);
const leftImage =
element.$.diff.$.diffTable.querySelector('td.left img');
const rightImage =
element.$.diff.$.diffTable.querySelector('td.right img');
assert.isOk(leftImage);
assert.isNotOk(rightImage);
done();
});
element.$.restAPI.getDiffPreferences().then(prefs => {
element.prefs = prefs;
element.reload();
});
});
test('does not render disallowed image type', done => {
const mockDiff = {
meta_a: {name: 'carrot.jpg', content_type: 'image/jpeg-evil',
lines: 560},
intraline_status: 'OK',
change_type: 'DELETED',
diff_header: [
'diff --git a/carrot.jpg b/carrot.jpg',
'index f9c2f2c..0000000 100644',
'--- a/carrot.jpg',
'+++ /dev/null',
'Binary files differ',
],
content: [{skip: 66}],
binary: true,
};
mockFile1.type = 'image/jpeg-evil';
sandbox.stub(element.$.restAPI, 'getDiff')
.returns(Promise.resolve(mockDiff));
element.addEventListener('render', () => {
// Recognizes that it should be an image diff.
assert.isTrue(element.isImageDiff);
assert.instanceOf(
element.$.diff.$.diffBuilder._builder, GrDiffBuilderImage);
const leftImage =
element.$.diff.$.diffTable.querySelector('td.left img');
assert.isNotOk(leftImage);
done();
});
element.$.restAPI.getDiffPreferences().then(prefs => {
element.prefs = prefs;
element.reload();
});
});
});
});
test('delegates cancel()', () => {
element = fixture('basic');
const stub = sandbox.stub(element.$.diff, 'cancel');
element.reload();
assert.isTrue(stub.calledOnce);
@@ -64,7 +478,6 @@ limitations under the License.
});
test('delegates getCursorStops()', () => {
element = fixture('basic');
const returnValue = [document.createElement('b')];
const stub = sandbox.stub(element.$.diff, 'getCursorStops')
.returns(returnValue);
@@ -74,7 +487,6 @@ limitations under the License.
});
test('delegates isRangeSelected()', () => {
element = fixture('basic');
const returnValue = true;
const stub = sandbox.stub(element.$.diff, 'isRangeSelected')
.returns(returnValue);
@@ -84,33 +496,66 @@ limitations under the License.
});
test('delegates toggleLeftDiff()', () => {
element = fixture('basic');
const stub = sandbox.stub(element.$.diff, 'toggleLeftDiff');
element.toggleLeftDiff();
assert.isTrue(stub.calledOnce);
assert.equal(stub.lastCall.args.length, 0);
});
test('delegates loadBlame()', () => {
element = fixture('basic');
const returnValue = Promise.resolve();
const stub = sandbox.stub(element.$.diff, 'loadBlame')
.returns(returnValue);
assert.equal(element.loadBlame(), returnValue);
assert.isTrue(stub.calledOnce);
assert.equal(stub.lastCall.args.length, 0);
});
suite('blame', () => {
setup(() => {
element = fixture('basic');
});
test('delegates clearBlame()', () => {
element = fixture('basic');
const stub = sandbox.stub(element.$.diff, 'clearBlame');
element.clearBlame();
assert.isTrue(stub.calledOnce);
assert.equal(stub.lastCall.args.length, 0);
test('clearBlame', () => {
element._blame = [];
const setBlameSpy = sandbox.spy(element.$.diff.$.diffBuilder, 'setBlame');
element.clearBlame();
assert.isNull(element._blame);
assert.isTrue(setBlameSpy.calledWithExactly(null));
assert.equal(element.isBlameLoaded, false);
});
test('loadBlame', () => {
const mockBlame = [{id: 'commit id', ranges: [{start: 1, end: 2}]}];
const showAlertStub = sinon.stub();
element.addEventListener('show-alert', showAlertStub);
const getBlameStub = sandbox.stub(element.$.restAPI, 'getBlame')
.returns(Promise.resolve(mockBlame));
element.changeNum = 42;
element.patchRange = {patchNum: 5, basePatchNum: 4};
element.path = 'foo/bar.baz';
return element.loadBlame().then(() => {
assert.isTrue(getBlameStub.calledWithExactly(
42, 5, 'foo/bar.baz', true));
assert.isFalse(showAlertStub.called);
assert.equal(element._blame, mockBlame);
assert.equal(element.isBlameLoaded, true);
});
});
test('loadBlame empty', () => {
const mockBlame = [];
const showAlertStub = sinon.stub();
element.addEventListener('show-alert', showAlertStub);
sandbox.stub(element.$.restAPI, 'getBlame')
.returns(Promise.resolve(mockBlame));
element.changeNum = 42;
element.patchRange = {patchNum: 5, basePatchNum: 4};
element.path = 'foo/bar.baz';
return element.loadBlame()
.then(() => {
assert.isTrue(false, 'Promise should not resolve');
})
.catch(() => {
assert.isTrue(showAlertStub.calledOnce);
assert.isNull(element._blame);
assert.equal(element.isBlameLoaded, false);
});
});
});
test('delegates getThreadEls()', () => {
element = fixture('basic');
const returnValue = [document.createElement('b')];
const stub = sandbox.stub(element.$.diff, 'getThreadEls')
.returns(returnValue);
@@ -120,7 +565,6 @@ limitations under the License.
});
test('delegates addDraftAtLine(el)', () => {
element = fixture('basic');
const param0 = document.createElement('b');
const stub = sandbox.stub(element.$.diff, 'addDraftAtLine');
element.addDraftAtLine(param0);
@@ -130,7 +574,6 @@ limitations under the License.
});
test('delegates clearDiffContent()', () => {
element = fixture('basic');
const stub = sandbox.stub(element.$.diff, 'clearDiffContent');
element.clearDiffContent();
assert.isTrue(stub.calledOnce);
@@ -138,7 +581,6 @@ limitations under the License.
});
test('delegates expandAllContext()', () => {
element = fixture('basic');
const stub = sandbox.stub(element.$.diff, 'expandAllContext');
element.expandAllContext();
assert.isTrue(stub.calledOnce);
@@ -146,101 +588,66 @@ limitations under the License.
});
test('passes in changeNum', () => {
element = fixture('basic');
const value = '12345';
element.changeNum = value;
assert.equal(element.$.diff.changeNum, value);
});
test('passes in noAutoRender', () => {
element = fixture('basic');
const value = true;
element.noAutoRender = value;
assert.equal(element.$.diff.noAutoRender, value);
});
test('passes in patchRange', () => {
element = fixture('basic');
const value = {patchNum: 'foo', basePatchNum: 'bar'};
element.patchRange = value;
assert.equal(element.$.diff.patchRange, value);
});
test('passes in path', () => {
element = fixture('basic');
const value = 'some/file/path';
element.path = value;
assert.equal(element.$.diff.path, value);
});
test('passes in prefs', () => {
element = fixture('basic');
const value = {};
element.prefs = value;
assert.equal(element.$.diff.prefs, value);
});
test('passes in projectConfig', () => {
element = fixture('basic');
const value = {};
element.projectConfig = value;
assert.equal(element.$.diff.projectConfig, value);
});
test('passes in changeNum', () => {
element = fixture('basic');
const value = '12345';
element.changeNum = value;
assert.equal(element.$.diff.changeNum, value);
});
test('passes in projectName', () => {
element = fixture('basic');
const value = 'Gerrit';
element.projectName = value;
assert.equal(element.$.diff.projectName, value);
});
test('passes in displayLine', () => {
element = fixture('basic');
const value = true;
element.displayLine = value;
assert.equal(element.$.diff.displayLine, value);
});
test('passes out isImageDiff', () => {
element = fixture('basic');
const value = true;
// isImageDiff is computed, so we cannot just set it.
sandbox.stub(element.$.diff, '_computeIsImageDiff').returns(value);
element.$.diff._diff = {left: [], right: [], content: []};
assert.equal(element.isImageDiff, value);
});
test('passes in commitRange', () => {
element = fixture('basic');
const value = {};
element.commitRange = value;
assert.equal(element.$.diff.commitRange, value);
});
test('passes in filesWeblinks', () => {
element = fixture('basic');
const value = {};
element.filesWeblinks = value;
assert.equal(element.$.diff.filesWeblinks, value);
});
test('passes out filesWeblinks', () => {
element = fixture('basic');
const value = {};
element.$.diff.filesWeblinks = value;
assert.equal(element.filesWeblinks, value);
});
test('passes in hidden', () => {
element = fixture('basic');
const value = true;
element.hidden = value;
assert.equal(element.$.diff.hidden, value);
@@ -248,53 +655,125 @@ limitations under the License.
});
test('passes in noRenderOnPrefsChange', () => {
element = fixture('basic');
const value = true;
element.noRenderOnPrefsChange = value;
assert.equal(element.$.diff.noRenderOnPrefsChange, value);
});
test('passes in comments', () => {
element = fixture('basic');
const value = {left: [], right: []};
element.comments = value;
assert.equal(element.$.diff.comments, value);
});
test('passes in lineWrapping', () => {
element = fixture('basic');
const value = true;
element.lineWrapping = value;
assert.equal(element.$.diff.lineWrapping, value);
});
test('passes in viewMode', () => {
element = fixture('basic');
const value = 'SIDE_BY_SIDE';
element.viewMode = value;
assert.equal(element.$.diff.viewMode, value);
});
test('passes in lineOfInterest', () => {
element = fixture('basic');
const value = {number: 123, leftSide: true};
element.lineOfInterest = value;
assert.equal(element.$.diff.lineOfInterest, value);
});
test('passes in showLoadFailure', () => {
element = fixture('basic');
const value = true;
element.showLoadFailure = value;
assert.equal(element.$.diff.showLoadFailure, value);
});
suite('_reportDiff', () => {
let reportStub;
test('passes out isBlameLoaded', () => {
element = fixture('basic');
const value = true;
sandbox.stub(element.$.diff, '_computeIsBlameLoaded').returns(value);
element.$.diff._blame = {};
assert.equal(element.isBlameLoaded, value);
setup(() => {
element = fixture('basic');
element.patchRange = {basePatchNum: 1};
reportStub = sandbox.stub(element.$.reporting, 'reportInteraction');
});
test('null and content-less', () => {
element._reportDiff(null);
assert.isFalse(reportStub.called);
element._reportDiff({});
assert.isFalse(reportStub.called);
});
test('diff w/ no delta', () => {
const diff = {
content: [
{ab: ['foo', 'bar']},
{ab: ['baz', 'foo']},
],
};
element._reportDiff(diff);
assert.isTrue(reportStub.calledOnce);
assert.equal(reportStub.lastCall.args[0], 'rebase-percent-zero');
assert.isUndefined(reportStub.lastCall.args[1]);
});
test('diff w/ no rebase delta', () => {
const diff = {
content: [
{ab: ['foo', 'bar']},
{a: ['baz', 'foo']},
{ab: ['foo', 'bar']},
{a: ['baz', 'foo'], b: ['bar', 'baz']},
{ab: ['foo', 'bar']},
{b: ['baz', 'foo']},
{ab: ['foo', 'bar']},
],
};
element._reportDiff(diff);
assert.isTrue(reportStub.calledOnce);
assert.equal(reportStub.lastCall.args[0], 'rebase-percent-zero');
assert.isUndefined(reportStub.lastCall.args[1]);
});
test('diff w/ some rebase delta', () => {
const diff = {
content: [
{ab: ['foo', 'bar']},
{a: ['baz', 'foo'], due_to_rebase: true},
{ab: ['foo', 'bar']},
{a: ['baz', 'foo'], b: ['bar', 'baz']},
{ab: ['foo', 'bar']},
{b: ['baz', 'foo'], due_to_rebase: true},
{ab: ['foo', 'bar']},
{a: ['baz', 'foo']},
],
};
element._reportDiff(diff);
assert.isTrue(reportStub.calledOnce);
assert.equal(reportStub.lastCall.args[0], 'rebase-percent-nonzero');
assert.strictEqual(reportStub.lastCall.args[1], 50);
});
test('diff w/ all rebase delta', () => {
const diff = {content: [{
a: ['foo', 'bar'],
b: ['baz', 'foo'],
due_to_rebase: true,
}]};
element._reportDiff(diff);
assert.isTrue(reportStub.calledOnce);
assert.equal(reportStub.lastCall.args[0], 'rebase-percent-nonzero');
assert.strictEqual(reportStub.lastCall.args[1], 100);
});
test('diff against parent event', () => {
element.patchRange.basePatchNum = 'PARENT';
const diff = {content: [{
a: ['foo', 'bar'],
b: ['baz', 'foo'],
}]};
element._reportDiff(diff);
assert.isTrue(reportStub.calledOnce);
assert.equal(reportStub.lastCall.args[0], 'diff-against-parent');
assert.isUndefined(reportStub.lastCall.args[1]);
});
});
});
</script>

24
polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
View File

@@ -18,9 +18,7 @@ limitations under the License.
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../../behaviors/gr-patch-set-behavior/gr-patch-set-behavior.html">
<link rel="import" href="../../../styles/shared-styles.html">
<link rel="import" href="../../core/gr-reporting/gr-reporting.html">
<link rel="import" href="../../shared/gr-button/gr-button.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
<link rel="import" href="../gr-diff-builder/gr-diff-builder.html">
<link rel="import" href="../gr-diff-comment-thread/gr-diff-comment-thread.html">
<link rel="import" href="../gr-diff-highlight/gr-diff-highlight.html">
@@ -270,26 +268,26 @@ limitations under the License.
<div>[[item]]</div>
</template>
</div>
<div class$="[[_computeContainerClass(_loggedIn, viewMode, displayLine)]]"
<div class$="[[_computeContainerClass(loggedIn, viewMode, displayLine)]]"
on-tap="_handleTap">
<gr-diff-selection diff="[[_diff]]">
<gr-diff-selection diff="[[diff]]">
<gr-diff-highlight
id="highlights"
logged-in="[[_loggedIn]]"
logged-in="[[loggedIn]]"
comments="{{comments}}">
<gr-diff-builder
id="diffBuilder"
comments="[[comments]]"
project-name="[[projectName]]"
diff="[[_diff]]"
diff="[[diff]]"
diff-path="[[path]]"
change-num="[[changeNum]]"
patch-num="[[patchRange.patchNum]]"
view-mode="[[viewMode]]"
line-wrapping="[[lineWrapping]]"
is-image-diff="[[isImageDiff]]"
base-image="[[_baseImage]]"
revision-image="[[_revisionImage]]"
base-image="[[baseImage]]"
revision-image="[[revisionImage]]"
parent-index="[[_parentIndex]]"
create-comment-fn="[[_createThreadGroupFn]]"
line-of-interest="[[lineOfInterest]]">
@@ -301,16 +299,16 @@ limitations under the License.
</gr-diff-highlight>
</gr-diff-selection>
</div>
<div class$="[[_computeNewlineWarningClass(_newlineWarning, _loading)]]">
<div class$="[[_computeNewlineWarningClass(_newlineWarning, loading)]]">
[[_newlineWarning]]
</div>
<div id="loadingError" class$="[[_computeErrorClass(_errorMessage)]]">
[[_errorMessage]]
<div id="loadingError" class$="[[_computeErrorClass(errorMessage)]]">
[[errorMessage]]
</div>
<div id="sizeWarning" class$="[[_computeWarningClass(_showWarning)]]">
<p>
Prevented render because "Whole file" is enabled and this diff is very
large (about [[_diffLength(_diff)]] lines).
large (about [[_diffLength(diff)]] lines).
</p>
<gr-button on-tap="_handleLimitedBypass">
Render with limited context
@@ -319,8 +317,6 @@ limitations under the License.
Render anyway (may be slow)
</gr-button>
</div>
<gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
<gr-reporting id="reporting" category="diff"></gr-reporting>
</template>
<script src="gr-diff-line.js"></script>
<script src="gr-diff-group.js"></script>

304
polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
View File

@@ -21,11 +21,6 @@
const ERR_COMMENT_ON_EDIT_BASE = 'You cannot comment on the base patch set ' +
'of an edit.';
const ERR_INVALID_LINE = 'Invalid line number: ';
const MSG_EMPTY_BLAME = 'No blame information for this diff.';
const EVENT_AGAINST_PARENT = 'diff-against-parent';
const EVENT_ZERO_REBASE = 'rebase-percent-zero';
const EVENT_NONZERO_REBASE = 'rebase-percent-nonzero';
const NO_NEWLINE_BASE = 'No newline at end of base file.';
const NO_NEWLINE_REVISION = 'No newline at end of revision file.';
@@ -64,6 +59,12 @@
* @event diff-comments-modified
*/
/**
* Fired when a draft is added or edited.
*
* @event draft-interaction
*/
properties: {
changeNum: String,
noAutoRender: {
@@ -88,15 +89,8 @@
},
isImageDiff: {
type: Boolean,
computed: '_computeIsImageDiff(_diff)',
notify: true,
},
commitRange: Object,
filesWeblinks: {
type: Object,
value() { return {}; },
notify: true,
},
hidden: {
type: Boolean,
reflectToAttribute: true,
@@ -123,38 +117,33 @@
*/
lineOfInterest: Object,
/**
* If the diff fails to load, show the failure message in the diff rather
* than bubbling the error up to the whole page. This is useful for when
* loading inline diffs because one diff failing need not mark the whole
* page with a failure.
*/
showLoadFailure: Boolean,
_loading: {
loading: {
type: Boolean,
value: false,
observer: '_loadingChanged',
},
_loggedIn: {
loggedIn: {
type: Boolean,
value: false,
},
_diff: Object,
diff: {
type: Object,
observer: '_diffChanged',
},
_diffHeaderItems: {
type: Array,
value: [],
computed: '_computeDiffHeaderItems(_diff.*)',
computed: '_computeDiffHeaderItems(diff.*)',
},
_diffTableClass: {
type: String,
value: '',
},
/** @type {?Object} */
_baseImage: Object,
baseImage: Object,
/** @type {?Object} */
_revisionImage: Object,
revisionImage: Object,
/**
* Whether the safety check for large diffs when whole-file is set has
@@ -172,20 +161,16 @@
_showWarning: Boolean,
/** @type {?string} */
_errorMessage: {
errorMessage: {
type: String,
value: null,
},
/** @type {?Object} */
_blame: {
blame: {
type: Object,
value: null,
},
isBlameLoaded: {
type: Boolean,
notify: true,
computed: '_computeIsBlameLoaded(_blame)',
observer: '_blameChanged',
},
_parentIndex: {
@@ -195,7 +180,7 @@
_newlineWarning: {
type: String,
computed: '_computeNewlineWarning(_diff)',
computed: '_computeNewlineWarning(diff)',
},
/**
@@ -220,63 +205,6 @@
'create-comment': '_handleCreateComment',
},
attached() {
this._getLoggedIn().then(loggedIn => {
this._loggedIn = loggedIn;
});
},
ready() {
if (this._canRender()) {
this.reload();
}
},
/** @return {!Promise} */
reload() {
this._loading = true;
this._errorMessage = null;
const diffRequest = this._getDiff()
.then(diff => {
this._reportDiff(diff);
return diff;
})
.catch(e => {
this._handleGetDiffError(e);
return null;
});
const assetRequest = diffRequest.then(diff => {
// If the diff is null, then it's failed to load.
if (!diff) { return null; }
return this._loadDiffAssets(diff);
});
return Promise.all([diffRequest, assetRequest])
.then(results => {
const diff = results[0];
if (!diff) {
return Promise.resolve();
}
this.filesWeblinks = this._getFilesWeblinks(diff);
this._diff = diff;
return new Promise(resolve => {
const callback = () => {
resolve();
this.removeEventListener('render', callback);
};
this.addEventListener('render', callback);
this._renderDiffTable();
});
})
.catch(err => {
console.warn('Error encountered loading diff:', err);
})
.then(() => { this._loading = false; });
},
/** Cancel any remaining diff builder rendering work. */
cancel() {
this.$.diffBuilder.cancel();
@@ -300,37 +228,13 @@
this.toggleClass('no-left');
},
/**
* Load and display blame information for the base of the diff.
* @return {Promise} A promise that resolves when blame finishes rendering.
*/
loadBlame() {
return this.$.restAPI.getBlame(this.changeNum, this.patchRange.patchNum,
this.path, true)
.then(blame => {
if (!blame.length) {
this.fire('show-alert', {message: MSG_EMPTY_BLAME});
return Promise.reject(MSG_EMPTY_BLAME);
}
this._blame = blame;
this.$.diffBuilder.setBlame(blame);
this.classList.add('showBlame');
});
},
_computeIsBlameLoaded(blame) {
return !!blame;
},
/**
* Unload blame information for the diff.
*/
clearBlame() {
this._blame = null;
this.$.diffBuilder.setBlame(null);
this.classList.remove('showBlame');
_blameChanged(newValue) {
this.$.diffBuilder.setBlame(newValue);
if (newValue) {
this.classList.add('showBlame');
} else {
this.classList.remove('showBlame');
}
},
_handleCommentSaveOrDiscard() {
@@ -338,12 +242,6 @@
{bubbles: true}));
},
/** @return {boolean}} */
_canRender() {
return !!this.changeNum && !!this.patchRange && !!this.path &&
!this.noAutoRender;
},
/** @return {!Array<!HTMLElement>} */
getThreadEls() {
let threads = [];
@@ -432,7 +330,7 @@
/** @return {boolean} */
_isValidElForComment(el) {
if (!this._loggedIn) {
if (!this.loggedIn) {
this.fire('show-auth-required');
return false;
}
@@ -461,7 +359,7 @@
* @param {!Object=} opt_range
*/
_createComment(lineEl, opt_lineNum, opt_side, opt_range) {
this.$.reporting.recordDraftInteraction();
this.dispatchEvent(new CustomEvent('draft-interaction', {bubbles: true}));
const contentText = this.$.diffBuilder.getContentByLineEl(lineEl);
const contentEl = contentText.parentElement;
const side = opt_side ||
@@ -681,7 +579,7 @@
_loadingChanged(newValue) {
if (newValue) {
this.cancel();
this.clearBlame();
this._blame = null;
this._safetyBypass = null;
this._showWarning = false;
this.clearDiffContent();
@@ -695,7 +593,7 @@
_prefsChanged(prefs) {
if (!prefs) { return; }
this.clearBlame();
this._blame = null;
const stylesToUpdate = {};
@@ -716,7 +614,13 @@
this.updateStyles(stylesToUpdate);
if (this._diff && this.comments && !this.noRenderOnPrefsChange) {
if (this.diff && this.comments && !this.noRenderOnPrefsChange) {
this._renderDiffTable();
}
},
_diffChanged(newValue) {
if (newValue) {
this._renderDiffTable();
}
},
@@ -727,7 +631,7 @@
return;
}
if (this.prefs.context === -1 &&
this._diffLength(this._diff) >= LARGE_DIFF_THRESHOLD_LINES &&
this._diffLength(this.diff) >= LARGE_DIFF_THRESHOLD_LINES &&
this._safetyBypass === null) {
this._showWarning = true;
this.dispatchEvent(new CustomEvent('render', {bubbles: true}));
@@ -752,138 +656,6 @@
this.$.diffTable.innerHTML = null;
},
_handleGetDiffError(response) {
// Loading the diff may respond with 409 if the file is too large. In this
// case, use a toast error..
if (response.status === 409) {
this.fire('server-error', {response});
return;
}
if (this.showLoadFailure) {
this._errorMessage = [
'Encountered error when loading the diff:',
response.status,
response.statusText,
].join(' ');
return;
}
this.fire('page-error', {response});
},
/** @return {!Promise<!Object>} */
_getDiff() {
// Wrap the diff request in a new promise so that the error handler
// rejects the promise, allowing the error to be handled in the .catch.
return new Promise((resolve, reject) => {
this.$.restAPI.getDiff(
this.changeNum,
this.patchRange.basePatchNum,
this.patchRange.patchNum,
this.path,
reject)
.then(resolve);
});
},
_getFilesWeblinks(diff) {
if (!this.commitRange) { return {}; }
return {
meta_a: Gerrit.Nav.getFileWebLinks(
this.projectName, this.commitRange.baseCommit, this.path,
{weblinks: diff && diff.meta_a && diff.meta_a.web_links}),
meta_b: Gerrit.Nav.getFileWebLinks(
this.projectName, this.commitRange.commit, this.path,
{weblinks: diff && diff.meta_b && diff.meta_b.web_links}),
};
},
/**
* Report info about the diff response.
*/
_reportDiff(diff) {
if (!diff || !diff.content) { return; }
// Count the delta lines stemming from normal deltas, and from
// due_to_rebase deltas.
let nonRebaseDelta = 0;
let rebaseDelta = 0;
diff.content.forEach(chunk => {
if (chunk.ab) { return; }
const deltaSize = Math.max(
chunk.a ? chunk.a.length : 0, chunk.b ? chunk.b.length : 0);
if (chunk.due_to_rebase) {
rebaseDelta += deltaSize;
} else {
nonRebaseDelta += deltaSize;
}
});
// Find the percent of the delta from due_to_rebase chunks rounded to two
// digits. Diffs with no delta are considered 0%.
const totalDelta = rebaseDelta + nonRebaseDelta;
const percentRebaseDelta = !totalDelta ? 0 :
Math.round(100 * rebaseDelta / totalDelta);
// Report the due_to_rebase percentage in the "diff" category when
// applicable.
if (this.patchRange.basePatchNum === 'PARENT') {
this.$.reporting.reportInteraction(EVENT_AGAINST_PARENT);
} else if (percentRebaseDelta === 0) {
this.$.reporting.reportInteraction(EVENT_ZERO_REBASE);
} else {
this.$.reporting.reportInteraction(EVENT_NONZERO_REBASE,
percentRebaseDelta);
}
},
/** @return {!Promise} */
_getLoggedIn() {
return this.$.restAPI.getLoggedIn();
},
/**
* @param {Object} diff
* @return {boolean}
*/
_computeIsImageDiff(diff) {
if (!diff) { return false; }
const isA = diff.meta_a &&
diff.meta_a.content_type.startsWith('image/');
const isB = diff.meta_b &&
diff.meta_b.content_type.startsWith('image/');
return !!(diff.binary && (isA || isB));
},
/**
* @param {Object} diff
* @return {!Promise}
*/
_loadDiffAssets(diff) {
if (this._computeIsImageDiff(diff)) {
return this._getImages(diff).then(images => {
this._baseImage = images.baseImage;
this._revisionImage = images.revisionImage;
});
} else {
this._baseImage = null;
this._revisionImage = null;
return Promise.resolve();
}
},
/**
* @param {Object} diff
* @return {!Promise}
*/
_getImages(diff) {
return this.$.restAPI.getImagesForDiff(this.changeNum, diff,
this.patchRange);
},
_projectConfigChanged(projectConfig) {
const threadEls = this.getThreadEls();
for (let i = 0; i < threadEls.length; i++) {

416
polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
View File

@@ -48,18 +48,8 @@ limitations under the License.
sandbox.restore();
});
test('reload cancels before network resolves', () => {
element = fixture('basic');
const cancelStub = sandbox.stub(element, 'cancel');
// Stub the network calls into requests that never resolve.
sandbox.stub(element, '_getDiff', () => new Promise(() => {}));
element.reload();
assert.isTrue(cancelStub.called);
});
test('cancel', () => {
element = fixture('basic');
const cancelStub = sandbox.stub(element.$.diffBuilder, 'cancel');
element.cancel();
assert.isTrue(cancelStub.calledOnce);
@@ -208,41 +198,6 @@ limitations under the License.
element.$$('.diffContainer').classList.contains('displayLine'));
});
test('loads files weblinks', () => {
const weblinksStub = sandbox.stub(Gerrit.Nav, '_generateWeblinks')
.returns({name: 'stubb', url: '#s'});
sandbox.stub(element.$.restAPI, 'getDiff').returns(Promise.resolve({
content: [],
}));
element.projectName = 'test-project';
element.path = 'test-path';
element.commitRange = {baseCommit: 'test-base', commit: 'test-commit'};
element.patchRange = {};
return element.reload().then(() => {
assert.isTrue(weblinksStub.calledTwice);
assert.isTrue(weblinksStub.firstCall.calledWith({
commit: 'test-base',
file: 'test-path',
options: {
weblinks: undefined,
},
repo: 'test-project',
type: Gerrit.Nav.WeblinkType.FILE}));
assert.isTrue(weblinksStub.secondCall.calledWith({
commit: 'test-commit',
file: 'test-path',
options: {
weblinks: undefined,
},
repo: 'test-project',
type: Gerrit.Nav.WeblinkType.FILE}));
assert.deepEqual(element.filesWeblinks, {
meta_a: [{name: 'stubb', url: '#s'}],
meta_b: [{name: 'stubb', url: '#s'}],
});
});
});
test('remove comment', () => {
element.comments = {
meta: {
@@ -408,66 +363,29 @@ limitations under the License.
'wsAAAAAAAAAAAAA/////w==',
type: 'image/bmp',
};
const 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',
};
const mockComments = {baseComments: [], comments: []};
sandbox.stub(element.$.restAPI, 'getCommitInfo')
.returns(Promise.resolve(mockCommit));
sandbox.stub(element.$.restAPI,
'getB64FileContents',
(changeId, patchNum, path, opt_parentIndex) => {
return Promise.resolve(opt_parentIndex === 1 ? mockFile1 :
mockFile2);
});
sandbox.stub(element.$.restAPI, '_getDiffComments')
.returns(Promise.resolve(mockComments));
sandbox.stub(element.$.restAPI, 'getDiffDrafts')
.returns(Promise.resolve(mockComments));
element.patchRange = {basePatchNum: 'PARENT', patchNum: 1};
element.comments = {left: [], right: []};
element.isImageDiff = true;
element.prefs = {
auto_hide_diff_table_header: true,
context: 10,
cursor_blink_rate: 0,
font_size: 12,
ignore_whitespace: 'IGNORE_NONE',
intraline_difference: true,
line_length: 100,
line_wrapping: false,
show_line_endings: true,
show_tabs: true,
show_whitespace_errors: true,
syntax_highlighting: true,
tab_size: 8,
theme: 'DEFAULT',
};
});
test('renders image diffs with same file name', done => {
const 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,
};
sandbox.stub(element.$.restAPI, 'getDiff')
.returns(Promise.resolve(mockDiff));
const rendered = () => {
// Recognizes that it should be an image diff.
assert.isTrue(element.isImageDiff);
@@ -522,10 +440,24 @@ limitations under the License.
element.addEventListener('render', rendered);
element.$.restAPI.getDiffPreferences().then(prefs => {
element.prefs = prefs;
element.reload();
});
element.baseImage = mockFile1;
element.revisionImage = mockFile2;
element.diff = {
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,
};
});
test('renders image diffs with a different file name', done => {
@@ -545,8 +477,6 @@ limitations under the License.
content: [{skip: 66}],
binary: true,
};
sandbox.stub(element.$.restAPI, 'getDiff')
.returns(Promise.resolve(mockDiff));
const rendered = () => {
// Recognizes that it should be an image diff.
@@ -604,10 +534,11 @@ limitations under the License.
element.addEventListener('render', rendered);
element.$.restAPI.getDiffPreferences().then(prefs => {
element.prefs = prefs;
element.reload();
});
element.baseImage = mockFile1;
element.baseImage._name = mockDiff.meta_a.name;
element.revisionImage = mockFile2;
element.revisionImage._name = mockDiff.meta_b.name;
element.diff = mockDiff;
});
test('renders added image', done => {
@@ -626,8 +557,6 @@ limitations under the License.
content: [{skip: 66}],
binary: true,
};
sandbox.stub(element.$.restAPI, 'getDiff')
.returns(Promise.resolve(mockDiff));
element.addEventListener('render', () => {
// Recognizes that it should be an image diff.
@@ -643,10 +572,8 @@ limitations under the License.
done();
});
element.$.restAPI.getDiffPreferences().then(prefs => {
element.prefs = prefs;
element.reload();
});
element.revisionImage = mockFile2;
element.diff = mockDiff;
});
test('renders removed image', done => {
@@ -665,8 +592,6 @@ limitations under the License.
content: [{skip: 66}],
binary: true,
};
sandbox.stub(element.$.restAPI, 'getDiff')
.returns(Promise.resolve(mockDiff));
element.addEventListener('render', () => {
// Recognizes that it should be an image diff.
@@ -682,10 +607,8 @@ limitations under the License.
done();
});
element.$.restAPI.getDiffPreferences().then(prefs => {
element.prefs = prefs;
element.reload();
});
element.baseImage = mockFile1;
element.diff = mockDiff;
});
test('does not render disallowed image type', done => {
@@ -706,9 +629,6 @@ limitations under the License.
};
mockFile1.type = 'image/jpeg-evil';
sandbox.stub(element.$.restAPI, 'getDiff')
.returns(Promise.resolve(mockDiff));
element.addEventListener('render', () => {
// Recognizes that it should be an image diff.
assert.isTrue(element.isImageDiff);
@@ -719,10 +639,8 @@ limitations under the License.
done();
});
element.$.restAPI.getDiffPreferences().then(prefs => {
element.prefs = prefs;
element.reload();
});
element.baseImage = mockFile1;
element.diff = mockDiff;
});
});
@@ -769,67 +687,10 @@ limitations under the License.
content.click();
});
test('_getDiff handles null diff responses', done => {
stub('gr-rest-api-interface', {
getDiff() { return Promise.resolve(null); },
});
element.changeNum = 123;
element.patchRange = {basePatchNum: 1, patchNum: 2};
element.path = 'file.txt';
element._getDiff().then(done);
});
test('reload resolves on error', () => {
const onErrStub = sandbox.stub(element, '_handleGetDiffError');
const error = {ok: false, status: 500};
sandbox.stub(element.$.restAPI, 'getDiff',
(changeNum, basePatchNum, patchNum, path, onErr) => {
onErr(error);
});
return element.reload().then(() => {
assert.isTrue(onErrStub.calledOnce);
});
});
suite('_handleGetDiffError', () => {
let serverErrorStub;
let pageErrorStub;
setup(() => {
serverErrorStub = sinon.stub();
element.addEventListener('server-error', serverErrorStub);
pageErrorStub = sinon.stub();
element.addEventListener('page-error', pageErrorStub);
});
test('page error on HTTP-409', () => {
element._handleGetDiffError({status: 409});
assert.isTrue(serverErrorStub.calledOnce);
assert.isFalse(pageErrorStub.called);
assert.isNotOk(element._errorMessage);
});
test('server error on non-HTTP-409', () => {
element._handleGetDiffError({status: 500});
assert.isFalse(serverErrorStub.called);
assert.isTrue(pageErrorStub.calledOnce);
assert.isNotOk(element._errorMessage);
});
test('error message if showLoadFailure', () => {
element.showLoadFailure = true;
element._handleGetDiffError({status: 500, statusText: 'Failure!'});
assert.isFalse(serverErrorStub.called);
assert.isFalse(pageErrorStub.called);
assert.equal(element._errorMessage,
'Encountered error when loading the diff: 500 Failure!');
});
});
suite('getCursorStops', () => {
const setupDiff = function() {
const mock = document.createElement('mock-diff-response');
element._diff = mock.diffResponse;
element.diff = mock.diffResponse;
element.comments = {
left: [],
right: [],
@@ -878,15 +739,8 @@ limitations under the License.
suite('logged in', () => {
let fakeLineEl;
setup(() => {
const getLoggedInPromise = Promise.resolve(true);
stub('gr-rest-api-interface', {
getLoggedIn() { return getLoggedInPromise; },
getPreferences() {
return Promise.resolve({time_format: 'HHMM_12'});
},
getAccountCapabilities() { return Promise.resolve(); },
});
element = fixture('basic');
element.loggedIn = true;
element.patchRange = {};
fakeLineEl = {
@@ -895,15 +749,11 @@ limitations under the License.
contains: sandbox.stub().returns(true),
},
};
return getLoggedInPromise;
});
test('addDraftAtLine', () => {
sandbox.stub(element, '_selectLine');
sandbox.stub(element, '_createComment');
const loggedInErrorSpy = sandbox.spy();
element.addEventListener('show-auth-required', loggedInErrorSpy);
assert.isFalse(loggedInErrorSpy.called);
element.addDraftAtLine(fakeLineEl);
assert.isTrue(element._createComment
.calledWithExactly(fakeLineEl, 42));
@@ -913,12 +763,9 @@ limitations under the License.
element.patchRange.basePatchNum = element.EDIT_NAME;
sandbox.stub(element, '_selectLine');
sandbox.stub(element, '_createComment');
const loggedInErrorSpy = sandbox.spy();
const alertSpy = sandbox.spy();
element.addEventListener('show-auth-required', loggedInErrorSpy);
element.addEventListener('show-alert', alertSpy);
element.addDraftAtLine(fakeLineEl);
assert.isFalse(loggedInErrorSpy.called);
assert.isTrue(alertSpy.called);
assert.isFalse(element._createComment.called);
});
@@ -928,19 +775,16 @@ limitations under the License.
element.patchRange.basePatchNum = element.PARENT_NAME;
sandbox.stub(element, '_selectLine');
sandbox.stub(element, '_createComment');
const loggedInErrorSpy = sandbox.spy();
const alertSpy = sandbox.spy();
element.addEventListener('show-auth-required', loggedInErrorSpy);
element.addEventListener('show-alert', alertSpy);
element.addDraftAtLine(fakeLineEl);
assert.isFalse(loggedInErrorSpy.called);
assert.isTrue(alertSpy.called);
assert.isFalse(element._createComment.called);
});
suite('change in preferences', () => {
setup(() => {
element._diff = {
element.diff = {
meta_a: {name: 'carrot.jpg', content_type: 'image/jpeg', lines: 66},
meta_b: {name: 'carrot.jpg', content_type: 'image/jpeg',
lines: 560},
@@ -1074,7 +918,8 @@ limitations under the License.
suite('diff header', () => {
setup(() => {
element._diff = {
element = fixture('basic');
element.diff = {
meta_a: {name: 'carrot.jpg', content_type: 'image/jpeg', lines: 66},
meta_b: {name: 'carrot.jpg', content_type: 'image/jpeg',
lines: 560},
@@ -1087,15 +932,15 @@ limitations under the License.
test('hidden', () => {
assert.equal(element._diffHeaderItems.length, 0);
element.push('_diff.diff_header', 'diff --git a/test.jpg b/test.jpg');
element.push('diff.diff_header', 'diff --git a/test.jpg b/test.jpg');
assert.equal(element._diffHeaderItems.length, 0);
element.push('_diff.diff_header', 'index 2adc47d..f9c2f2c 100644');
element.push('diff.diff_header', 'index 2adc47d..f9c2f2c 100644');
assert.equal(element._diffHeaderItems.length, 0);
element.push('_diff.diff_header', '--- a/test.jpg');
element.push('diff.diff_header', '--- a/test.jpg');
assert.equal(element._diffHeaderItems.length, 0);
element.push('_diff.diff_header', '+++ b/test.jpg');
element.push('diff.diff_header', '+++ b/test.jpg');
assert.equal(element._diffHeaderItems.length, 0);
element.push('_diff.diff_header', 'test');
element.push('diff.diff_header', 'test');
assert.equal(element._diffHeaderItems.length, 1);
flushAsynchronousOperations();
@@ -1103,13 +948,13 @@ limitations under the License.
});
test('binary files', () => {
element._diff.binary = true;
element.diff.binary = true;
assert.equal(element._diffHeaderItems.length, 0);
element.push('_diff.diff_header', 'diff --git a/test.jpg b/test.jpg');
element.push('diff.diff_header', 'diff --git a/test.jpg b/test.jpg');
assert.equal(element._diffHeaderItems.length, 0);
element.push('_diff.diff_header', 'test');
element.push('diff.diff_header', 'test');
assert.equal(element._diffHeaderItems.length, 1);
element.push('_diff.diff_header', 'Binary files differ');
element.push('diff.diff_header', 'Binary files differ');
assert.equal(element._diffHeaderItems.length, 1);
});
});
@@ -1126,7 +971,7 @@ limitations under the License.
new CustomEvent('render', {bubbles: true}));
});
const mock = document.createElement('mock-diff-response');
element._diff = mock.diffResponse;
element.diff = mock.diffResponse;
element.comments = {left: [], right: []};
element.noRenderOnPrefsChange = true;
});
@@ -1171,144 +1016,19 @@ limitations under the License.
element = fixture('basic');
});
test('clearBlame', () => {
element._blame = [];
test('unsetting', () => {
element.blame = [];
const setBlameSpy = sandbox.spy(element.$.diffBuilder, 'setBlame');
element.classList.add('showBlame');
element.clearBlame();
assert.isNull(element._blame);
element.blame = null;
assert.isTrue(setBlameSpy.calledWithExactly(null));
assert.isFalse(element.classList.contains('showBlame'));
});
test('loadBlame', () => {
test('setting', () => {
const mockBlame = [{id: 'commit id', ranges: [{start: 1, end: 2}]}];
const showAlertStub = sinon.stub();
element.addEventListener('show-alert', showAlertStub);
const getBlameStub = sandbox.stub(element.$.restAPI, 'getBlame')
.returns(Promise.resolve(mockBlame));
element.changeNum = 42;
element.patchRange = {patchNum: 5, basePatchNum: 4};
element.path = 'foo/bar.baz';
return element.loadBlame().then(() => {
assert.isTrue(getBlameStub.calledWithExactly(
42, 5, 'foo/bar.baz', true));
assert.isFalse(showAlertStub.called);
assert.equal(element._blame, mockBlame);
assert.isTrue(element.classList.contains('showBlame'));
});
});
test('loadBlame empty', () => {
const mockBlame = [];
const showAlertStub = sinon.stub();
element.addEventListener('show-alert', showAlertStub);
sandbox.stub(element.$.restAPI, 'getBlame')
.returns(Promise.resolve(mockBlame));
element.changeNum = 42;
element.patchRange = {patchNum: 5, basePatchNum: 4};
element.path = 'foo/bar.baz';
return element.loadBlame()
.then(() => {
assert.isTrue(false, 'Promise should not resolve');
})
.catch(() => {
assert.isTrue(showAlertStub.calledOnce);
assert.isNull(element._blame);
assert.isFalse(element.classList.contains('showBlame'));
});
});
});
suite('_reportDiff', () => {
let reportStub;
setup(() => {
element = fixture('basic');
element.patchRange = {basePatchNum: 1};
reportStub = sandbox.stub(element.$.reporting, 'reportInteraction');
});
test('null and content-less', () => {
element._reportDiff(null);
assert.isFalse(reportStub.called);
element._reportDiff({});
assert.isFalse(reportStub.called);
});
test('diff w/ no delta', () => {
const diff = {
content: [
{ab: ['foo', 'bar']},
{ab: ['baz', 'foo']},
],
};
element._reportDiff(diff);
assert.isTrue(reportStub.calledOnce);
assert.equal(reportStub.lastCall.args[0], 'rebase-percent-zero');
assert.isUndefined(reportStub.lastCall.args[1]);
});
test('diff w/ no rebase delta', () => {
const diff = {
content: [
{ab: ['foo', 'bar']},
{a: ['baz', 'foo']},
{ab: ['foo', 'bar']},
{a: ['baz', 'foo'], b: ['bar', 'baz']},
{ab: ['foo', 'bar']},
{b: ['baz', 'foo']},
{ab: ['foo', 'bar']},
],
};
element._reportDiff(diff);
assert.isTrue(reportStub.calledOnce);
assert.equal(reportStub.lastCall.args[0], 'rebase-percent-zero');
assert.isUndefined(reportStub.lastCall.args[1]);
});
test('diff w/ some rebase delta', () => {
const diff = {
content: [
{ab: ['foo', 'bar']},
{a: ['baz', 'foo'], due_to_rebase: true},
{ab: ['foo', 'bar']},
{a: ['baz', 'foo'], b: ['bar', 'baz']},
{ab: ['foo', 'bar']},
{b: ['baz', 'foo'], due_to_rebase: true},
{ab: ['foo', 'bar']},
{a: ['baz', 'foo']},
],
};
element._reportDiff(diff);
assert.isTrue(reportStub.calledOnce);
assert.equal(reportStub.lastCall.args[0], 'rebase-percent-nonzero');
assert.strictEqual(reportStub.lastCall.args[1], 50);
});
test('diff w/ all rebase delta', () => {
const diff = {content: [{
a: ['foo', 'bar'],
b: ['baz', 'foo'],
due_to_rebase: true,
}]};
element._reportDiff(diff);
assert.isTrue(reportStub.calledOnce);
assert.equal(reportStub.lastCall.args[0], 'rebase-percent-nonzero');
assert.strictEqual(reportStub.lastCall.args[1], 100);
});
test('diff against parent event', () => {
element.patchRange.basePatchNum = 'PARENT';
const diff = {content: [{
a: ['foo', 'bar'],
b: ['baz', 'foo'],
}]};
element._reportDiff(diff);
assert.isTrue(reportStub.calledOnce);
assert.equal(reportStub.lastCall.args[0], 'diff-against-parent');
assert.isUndefined(reportStub.lastCall.args[1]);
element.blame = mockBlame;
assert.isTrue(element.classList.contains('showBlame'));
});
});