Move all server calls to gr-diff-host
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
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
index 1aba403..a8ed5bc 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -18,9 +18,7 @@
<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 @@
<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 @@
</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 @@
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>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
index 881d97f..4f0a73b 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -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++) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
index dda3ca6..0274fae 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
@@ -48,18 +48,8 @@
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 @@
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 @@
'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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
};
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 @@
done();
});
- element.$.restAPI.getDiffPreferences().then(prefs => {
- element.prefs = prefs;
- element.reload();
- });
+ element.baseImage = mockFile1;
+ element.diff = mockDiff;
});
});
@@ -769,67 +687,10 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
});
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 @@
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 @@
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'));
});
});
