diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js index 367655e932..d7a1537a3d 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js +++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js @@ -312,22 +312,7 @@ } this._coverageRanges = []; - const {changeNum, path, patchRange: {basePatchNum, patchNum}} = this; - this.$.jsAPI.getCoverageRanges( - changeNum, path, basePatchNum, patchNum - ). - then(coverageRanges => { - if (changeNum !== this.changeNum || - path !== this.path || - basePatchNum !== this.patchRange.basePatchNum || - patchNum !== this.patchRange.patchNum) { - return; - } - this._coverageRanges = coverageRanges; - }).catch(err => { - console.warn('Loading coverage ranges failed: ', err); - }); - + this._getCoverageData(); const diffRequest = this._getDiff() .then(diff => { this._loadedWhitespaceLevel = whitespaceLevel; @@ -346,7 +331,7 @@ return this._loadDiffAssets(diff); }); - // Not waiting for getCoverageRanges intentionally as + // Not waiting for coverage ranges intentionally as // plugin loading should not block the content rendering return Promise.all([diffRequest, assetRequest]) .then(results => { @@ -387,6 +372,49 @@ .then(() => { this._loading = false; }); } + _getCoverageData() { + const {changeNum, path, patchRange: {basePatchNum, patchNum}} = this; + this.$.jsAPI.getCoverageAnnotationApi(). + then(coverageAnnotationApi => { + if (!coverageAnnotationApi) return; + const provider = coverageAnnotationApi.getCoverageProvider(); + return provider(changeNum, path, basePatchNum, patchNum) + .then(coverageRanges => { + if (!coverageRanges || + changeNum !== this.changeNum || + path !== this.path || + basePatchNum !== this.patchRange.basePatchNum || + patchNum !== this.patchRange.patchNum) { + return; + } + + const existingCoverageRanges = this._coverageRanges; + this._coverageRanges = coverageRanges; + + // Notify with existing coverage ranges + // in case there is some existing coverage data that needs to be removed + existingCoverageRanges.forEach(range => { + coverageAnnotationApi.notify( + path, + range.code_range.start_line, + range.code_range.end_line, + range.side); + }); + + // Notify with new coverage data + coverageRanges.forEach(range => { + coverageAnnotationApi.notify( + path, + range.code_range.start_line, + range.code_range.end_line, + range.side); + }); + }); + }).catch(err => { + console.warn('Loading coverage ranges failed: ', err); + }); + } + _getFilesWeblinks(diff) { if (!this.commitRange) { return {}; diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html index ea944412ef..1c707479e1 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html +++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html @@ -1429,5 +1429,69 @@ limitations under the License. assert.isFalse(element.$.syntaxLayer.enabled); }); }); + + suite('coverage layer', () => { + let notifyStub; + setup(() => { + notifyStub = sinon.stub(); + stub('gr-js-api-interface', { + getCoverageAnnotationApi() { + return Promise.resolve({ + notify: notifyStub, + getCoverageProvider() { + return () => Promise.resolve([ + { + type: 'COVERED', + side: 'right', + code_range: { + start_line: 1, + end_line: 2, + }, + }, + { + type: 'NOT_COVERED', + side: 'right', + code_range: { + start_line: 3, + end_line: 4, + }, + }, + ]); + }, + }); + }, + }); + element = fixture('basic'); + const prefs = { + line_length: 10, + show_tabs: true, + tab_size: 4, + context: -1, + }; + element.diff = { + content: [{ + a: ['foo'], + }], + }; + element.patchRange = {}; + element.prefs = prefs; + }); + + test('getCoverageAnnotationApi should be called', done => { + element.reload(); + flush(() => { + assert.isTrue(element.$.jsAPI.getCoverageAnnotationApi.calledOnce); + done(); + }); + }); + + test('coverageRangeChanged should be called', done => { + element.reload(); + flush(() => { + assert.equal(notifyStub.callCount, 2); + done(); + }); + }); + }); }); diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface.js b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface.js index ae08007a13..d59f9b15c4 100644 --- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface.js +++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface.js @@ -253,44 +253,15 @@ * Retrieves coverage data possibly provided by a plugin. * * Will wait for plugins to be loaded. If multiple plugins offer a coverage - * provider, the first one is used. If no plugin offers a coverage provider, - * will resolve to []. + * provider, the first one is returned. If no plugin offers a coverage provider, + * will resolve to null. * - * @param {string|number} changeNum - * @param {string} path - * @param {string|number} basePatchNum - * @param {string|number} patchNum - * @return {!Promise>} + * @return {!Promise} */ - getCoverageRanges(changeNum, path, basePatchNum, patchNum) { - return Gerrit.awaitPluginsLoaded().then(() => { - for (const annotationApi of - this._getEventCallbacks(EventType.ANNOTATE_DIFF)) { - const provider = annotationApi.getCoverageProvider(); - // Only one coverage provider makes sense. If there are more, then we - // simply ignore them. - if (provider) { - return annotationApi; - } - } - return null; - }).then(annotationApi => { - if (!annotationApi) return []; - const provider = annotationApi.getCoverageProvider(); - return provider(changeNum, path, basePatchNum, patchNum) - .then(ranges => { - ranges = ranges || []; - // Notify with the coverage data. - ranges.forEach(range => { - annotationApi.notify( - path, - range.code_range.start_line, - range.code_range.end_line, - range.side); - }); - return ranges; - }); - }); + getCoverageAnnotationApi() { + return Gerrit.awaitPluginsLoaded() + .then(() => this._getEventCallbacks(EventType.ANNOTATE_DIFF) + .find(api => api.getCoverageProvider())); } getAdminMenuLinks() {