+2017-03-19 Ryosuke Niwa <rniwa@webkit.org>
+
+ Charts page show an inconsistent list of revisions for Git and Subversion
+ https://bugs.webkit.org/show_bug.cgi?id=169888
+
+ Reviewed by Andreas Kling.
+
+ With Git, CommitLogViewer was showing the list of revisions including the starting hash,
+ which was the last data point's revision instead of all revisions after the last data point.
+
+ Fixed the bug by always specifying the revision at the last data point in both Subversion
+ and Git and then making /api/commits/<repository>/?from=X&to=Y exclude the first revision.
+ For clarity, "from" and "to" query parameters have been renamed to "precedingRevision" and
+ "lastRevision" respectively.
+
+ We also no longer adds 1 to the starting revision of Subversion-like starting revisions. e.g.
+ when the last data point was at r1234, new data point is at r1250, the label is now "r1234-r1250"
+ instead of "r1235-r1250".
+
+ * browser-tests/chart-revision-range-tests.js: Fixed the tests since revisionList no longer
+ specifies from/to revisions.
+ * browser-tests/commit-log-viewer-tests.js: Added. Added tests for CommitLogViewer.
+ * browser-tests/index.html: Include the new test. Also use a local copy of mocha.js/css.
+
+ * public/api/commits.php:
+ (main): Renamed "from" and "to" query parameters.
+
+ * public/include/commit-log-fetcher.php:
+ (CommitLogFetcher::fetch_between): Added a check that commit time should either be specified
+ in both rows or not specified in either. Also reject when before_first_revision is identical
+ or after last_revision instead of re-ordering them since it no longer makes sense to do so with
+ new query parameter names.
+
+ * public/v3/components/base.js:
+ (ComponentBase._addContentToElement): Use Array.isArray instead of instanceof. It's resilient
+ againt realm (global object) differences.
+
+ * public/v3/components/chart-pane-base.js:
+ (ChartPaneBase.prototype._updateCommitLogViewer): No longer calls enqueueToRender on this since
+ CommitLogViewer does that on its own now.
+ (ChartPaneBase.prototype.render): Juse use this._openRepository instead of relying on CommitLogViewer
+ to remember which repository is current. This was the only use of currentRepository.
+
+ * public/v3/components/commit-log-viewer.js:
+ (CommitLogViewer):
+ (CommitLogViewer.prototype.currentRepository): Deleted.
+ (CommitLogViewer.prototype.view):
+ (CommitLogViewer.prototype._fetchCommitLogs): Modernized and extracted from view to make it lazy.
+ Call fetchForSingleRevision when precedingRevision is not specified or it's identical to lastRevision
+ since the generic JSON API no longer supports being called with the identical revisions.
+ (CommitLogViewer.prototype.render): Modernized & simplified the code.
+ (CommitLogViewer.prototype._renderCommitList): Extracted from render to make it lazy.
+ (CommitLogViewer.htmlTemplate): Add ID on caption & tbody so that they're more easily addressable.
+ (CommitLogViewer.cssTemplate):
+
+ * public/v3/models/commit-log.js:
+ (CommitLog.prototype.diff): No longer includes from/to revisions in the result. Also avoid adding
+ 1 to a Subversion-like starting revision for creating the label. See above. But we still do this
+ for forming URLs due to the way tools like Trac work with Subversion revisions.
+ (CommitLog.fetchBetweenRevisions): Rewritten using DataModel.prototype.cachedFetch with FIXME for
+ what this function is supposed to be doing.
+ (CommitLog._cachedCommitLogs): Deleted.
+ (CommitLog.fetchForSingleRevision): Added.
+ (CommitLog._constructFromRawData): Added.
+
+ * public/v3/models/data-model.js:
+ (DataModelObject.cachedFetch): Don't parse query values as an integer. Just URL-escape them.
+
+ * public/v3/remote.js:
+ (BrowserRemoteAPI.prototype.sendHttpRequest): Fixed a typo.
+
+ * server-tests/api-commits-tests.js: Renamed from api-commits.js. Updated the existing tests to
+ use new query parameters and added more test cases.
+
+ * unit-tests/commit-log-tests.js: Updated the test cases now that CommitLog.prototype.diff no longer
+ includes from/to values. They're computed in ChartRevisionRange instead.
+
2017-03-20 Ryosuke Niwa <rniwa@webkit.org>
Fix os-build-fetcher.js and subprocess.js to make them work
expect(revisionList[0].repository.label()).to.be('SomeApp');
expect(revisionList[0].label).to.be('r4006');
- expect(revisionList[0].from).to.be(null);
- expect(revisionList[0].to).to.be('4006');
expect(revisionList[1].repository.label()).to.be('macOS');
expect(revisionList[1].label).to.be('15C50');
- expect(revisionList[1].from).to.be(null);
- expect(revisionList[1].to).to.be('15C50');
})
});
});
expect(revisionList.length).to.be(2);
expect(revisionList[0].repository.label()).to.be('SomeApp');
- expect(revisionList[0].label).to.be('r4005-r4006');
- expect(revisionList[0].from).to.be('4005');
- expect(revisionList[0].to).to.be('4006');
+ expect(revisionList[0].label).to.be('r4004-r4006');
expect(revisionList[1].repository.label()).to.be('macOS');
expect(revisionList[1].label).to.be('15C50');
- expect(revisionList[1].from).to.be(null);
- expect(revisionList[1].to).to.be('15C50');
chart.setIndicator(1004, true); // Across macOS change.
expect(revisionList.length).to.be(2);
expect(revisionList[0].repository.label()).to.be('SomeApp');
- expect(revisionList[0].label).to.be('r4004-r4004');
- expect(revisionList[0].from).to.be('4004');
- expect(revisionList[0].to).to.be('4004');
+ expect(revisionList[0].label).to.be('r4003-r4004');
expect(revisionList[1].repository.label()).to.be('macOS');
expect(revisionList[1].label).to.be('15B42 - 15C50');
- expect(revisionList[1].from).to.be('15B42');
- expect(revisionList[1].to).to.be('15C50');
});
});
expect(revisionList.length).to.be(2);
expect(revisionList[0].repository.label()).to.be('SomeApp');
- expect(revisionList[0].label).to.be('r4003-r4004'); // 4002 and 4005 are outliers and skipped.
- expect(revisionList[0].from).to.be('4003');
- expect(revisionList[0].to).to.be('4004');
+ expect(revisionList[0].label).to.be('r4002-r4004'); // 4001 and 4005 are outliers and skipped.
expect(revisionList[1].repository.label()).to.be('macOS');
expect(revisionList[1].label).to.be('15B42 - 15C50');
- expect(revisionList[1].from).to.be('15B42');
- expect(revisionList[1].to).to.be('15C50');
});
});
});
--- /dev/null
+describe('CommitLogViewer', () => {
+
+ function importCommitLogViewer(context)
+ {
+ const scripts = [
+ '../shared/statistics.js',
+ 'instrumentation.js',
+ 'lazily-evaluated-function.js',
+ 'models/data-model.js',
+ 'models/repository.js',
+ 'models/commit-set.js',
+ 'models/commit-log.js',
+ 'components/base.js',
+ 'components/spinner-icon.js',
+ 'components/commit-log-viewer.js'];
+ return context.importScripts(scripts, 'ComponentBase', 'CommitLogViewer', 'Repository', 'CommitLog', 'RemoteAPI').then(() => {
+ return context.symbols.CommitLogViewer;
+ });
+ }
+
+ const webkitCommit210949 = {
+ "id": "185334",
+ "revision": "210949",
+ "previousCommit": null,
+ "time": +new Date("2017-01-20T03:23:50.645Z"),
+ "authorName": "Chris Dumez",
+ "authorEmail": "cdumez@apple.com",
+ "message": "some message",
+ };
+
+ const webkitCommit210950 = {
+ "id": "185338",
+ "revision": "210950",
+ "previousCommit": null,
+ "time": +new Date("2017-01-20T03:49:37.887Z"),
+ "authorName": "Commit Queue",
+ "authorEmail": "commit-queue@webkit.org",
+ "message": "another message",
+ };
+
+ it('should initially be empty with no spinner', () => {
+ const context = new BrowsingContext();
+ return importCommitLogViewer(context).then((CommitLogViewer) => {
+ const viewer = new CommitLogViewer;
+ context.document.body.appendChild(viewer.element());
+ viewer.enqueueToRender();
+ return waitForComponentsToRender(context).then(() => {
+ expect(viewer.content('spinner-container').offsetHeight).to.be(0);
+ expect(viewer.content('commits-list').matches(':empty')).to.be(true);
+ expect(viewer.content('repository-name').matches(':empty')).to.be(true);
+ });
+ });
+ });
+
+ it('should show the repository name and the spinner once it started fetching the list of commits ', () => {
+ const context = new BrowsingContext();
+ return importCommitLogViewer(context).then((CommitLogViewer) => {
+ const Repository = context.symbols.Repository;
+ const SpinnerIcon = context.symbols.SpinnerIcon;
+ const ComponentBase = context.symbols.ComponentBase;
+ const RemoteAPI = context.symbols.RemoteAPI;
+ const viewer = new CommitLogViewer;
+ const webkit = new Repository(1, {name: 'WebKit'});
+ context.document.body.appendChild(viewer.element());
+ viewer.enqueueToRender();
+ return waitForComponentsToRender(context).then(() => {
+ expect(viewer.content('spinner-container').offsetHeight).to.be(0);
+ expect(viewer.content('commits-list').matches(':empty')).to.be(true);
+ expect(viewer.content('repository-name').matches(':empty')).to.be(true);
+ expect(RemoteAPI.requests.length, 0);
+ viewer.view(webkit, '210948', '210950');
+ expect(RemoteAPI.requests.length, 1);
+ return waitForComponentsToRender(context);
+ }).then(() => {
+ expect(viewer.content('spinner-container').offsetHeight).to.not.be(0);
+ expect(viewer.content('commits-list').matches(':empty')).to.be(true);
+ expect(viewer.content('repository-name').matches(':empty')).to.be(false);
+ expect(viewer.content('repository-name').textContent).to.contain('WebKit');
+ });
+ });
+ });
+
+ it('should show the repository name, the list of commits, and hide the spinner once the list of commits are fetched', () => {
+ const context = new BrowsingContext();
+ return importCommitLogViewer(context).then((CommitLogViewer) => {
+ const Repository = context.symbols.Repository;
+ const SpinnerIcon = context.symbols.SpinnerIcon;
+ const ComponentBase = context.symbols.ComponentBase;
+ const requests = context.symbols.RemoteAPI.requests;
+ const viewer = new CommitLogViewer;
+ const webkit = new Repository(1, {name: 'WebKit'});
+ context.document.body.appendChild(viewer.element());
+ viewer.enqueueToRender();
+ return waitForComponentsToRender(context).then(() => {
+ viewer.view(webkit, '210948', '210950');
+ return waitForComponentsToRender(context);
+ }).then(() => {
+ expect(viewer.content('spinner-container').offsetHeight).to.not.be(0);
+ expect(viewer.content('commits-list').matches(':empty')).to.be(true);
+ expect(viewer.content('repository-name').matches(':empty')).to.be(false);
+ expect(viewer.content('repository-name').textContent).to.contain('WebKit');
+ expect(requests.length).to.be(1);
+ expect(requests[0].url).to.be('/api/commits/1/?precedingRevision=210948&lastRevision=210950');
+ requests[0].resolve({
+ "status": "OK",
+ "commits": [webkitCommit210949, webkitCommit210950],
+ });
+ return waitForComponentsToRender(context);
+ }).then(() => {
+ expect(viewer.content('spinner-container').offsetHeight).to.be(0);
+ expect(viewer.content('commits-list').matches(':empty')).to.be(false);
+ expect(viewer.content('commits-list').textContent).to.contain('r210949');
+ expect(viewer.content('commits-list').textContent).to.contain('Chris Dumez');
+ expect(viewer.content('commits-list').textContent).to.contain('r210950');
+ expect(viewer.content('commits-list').textContent).to.contain('Commit Queue');
+ expect(viewer.content('repository-name').matches(':empty')).to.be(false);
+ expect(viewer.content('repository-name').textContent).to.contain('WebKit');
+ expect(viewer.content('commits-list').querySelector('a')).to.be(null);
+ });
+ });
+ });
+
+});
<!DOCTYPE html>
<html>
<head>
+<meta charset="utf-8">
<title>In-Browser Tests for Performance Dashboard</title>
-<link rel="stylesheet" href="https://cdn.rawgit.com/mochajs/mocha/2.2.5/mocha.css">
-<script src="https://cdnjs.cloudflare.com/ajax/libs/mocha/2.2.5/mocha.js"></script>
+<link rel="stylesheet" href="../node_modules/mocha/mocha.css">
+<script src="../node_modules/mocha/mocha.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/expect.js/0.2.0/expect.min.js"></script>
<script>
<script src="interactive-time-series-chart-tests.js"></script>
<script src="chart-status-evaluator-tests.js"></script>
<script src="chart-revision-range-tests.js"></script>
+<script src="commit-log-viewer-tests.js"></script>
<script>
afterEach(() => {
$commits = array();
if (!$filter) {
$keyword = array_get($_GET, 'keyword'); // V2 UI compatibility.
- $from = array_get($_GET, 'from');
- $to = array_get($_GET, 'to');
- $commits = $fetcher->fetch_between($repository_id, $from, $to, $keyword);
+ $preceding_revision = array_get($_GET, 'precedingRevision');
+ $last_revision = array_get($_GET, 'lastRevision');
+ $commits = $fetcher->fetch_between($repository_id, $preceding_revision, $last_revision, $keyword);
} else if ($filter == 'oldest') {
$commits = $fetcher->fetch_oldest($repository_id);
} else if ($filter == 'latest') {
return $repository_row['repository_id'];
}
- function fetch_between($repository_id, $first, $second, $keyword = NULL)
+ function fetch_between($repository_id, $before_first_revision, $last_revision, $keyword = NULL)
{
$statements = 'SELECT commit_id as "id",
commit_revision as "revision",
WHERE commit_repository = $1 AND commit_reported = true';
$values = array($repository_id);
- if ($first && $second) {
- $first_commit = $this->commit_for_revision($repository_id, $first);
- $second_commit = $this->commit_for_revision($repository_id, $second);
- $first = $first_commit['commit_time'];
- $second = $second_commit['commit_time'];
- $column_name = 'commit_time';
- if (!$first || !$second) {
- $first = $first_commit['commit_order'];
- $second = $second_commit['commit_order'];
+ if ($before_first_revision && $last_revision) {
+ $preceding_commit = $this->commit_for_revision($repository_id, $before_first_revision);
+ $last_commit = $this->commit_for_revision($repository_id, $last_revision);
+
+ $preceding = $preceding_commit['commit_time'];
+ $last = $last_commit['commit_time'];
+ if (!$preceding != !$last)
+ exit_with_error('InconsistentCommits');
+ if ($preceding)
+ $column_name = 'commit_time';
+ else {
+ $preceding = $preceding_commit['commit_order'];
+ $last = $last_commit['commit_order'];
$column_name = 'commit_order';
+ if (!$preceding || !$last)
+ return array();
}
+ if ($preceding >= $last)
+ exit_with_error('InvalidCommitRange');
- $in_order = $first < $second;
- array_push($values, $in_order ? $first : $second);
- $statements .= ' AND ' . $column_name . ' >= $' . count($values);
- array_push($values, $in_order ? $second : $first);
+ array_push($values, $preceding);
+ $statements .= ' AND ' . $column_name . ' > $' . count($values);
+ array_push($values, $last);
$statements .= ' AND ' . $column_name . ' <= $' . count($values);
}
static _addContentToElement(element, content)
{
- if (content instanceof Array) {
+ if (Array.isArray(content)) {
for (var nestedChild of content)
this._addContentToElement(element, nestedChild);
} else if (content instanceof Node)
_updateCommitLogViewer()
{
const range = this._revisionRange.rangeForRepository(this._openRepository);
- const updateRendering = () => { this.enqueueToRender(); };
- this._commitLogViewer.view(this._openRepository, range.from, range.to).then(updateRendering);
- updateRendering();
+ this._commitLogViewer.view(this._openRepository, range.from, range.to);
+ this.enqueueToRender();
}
_openAnalysisTask(annotation)
this._mainChartStatus.enqueueToRender();
var body = this.content().querySelector('.chart-pane-body');
- if (this._commitLogViewer && this._commitLogViewer.currentRepository()) {
+ if (this._openRepository) {
body.classList.add('has-second-sidebar');
this._commitLogViewer.enqueueToRender();
} else
constructor()
{
super('commit-log-viewer');
+ this._lazilyFetchCommitLogs = new LazilyEvaluatedFunction(this._fetchCommitLogs.bind(this));
this._repository = null;
this._fetchingPromise = null;
this._commits = null;
- this._from = null;
- this._to = null;
+ this._renderCommitListLazily = new LazilyEvaluatedFunction(this._renderCommitList.bind(this));
}
- currentRepository() { return this._repository; }
-
- view(repository, from, to)
+ view(repository, precedingRevision, lastRevision)
{
- if (this._repository == repository && this._from == from && this._to == to)
- return Promise.resolve(null);
+ this._lazilyFetchCommitLogs.evaluate(repository, precedingRevision, lastRevision);
+ }
+ _fetchCommitLogs(repository, precedingRevision, lastRevision)
+ {
+ this._fetchingPromise = null;
this._commits = null;
- this._repository = repository;
- this._from = from;
- this._to = to;
- if (!repository) {
- this._fetchingPromise = null;
+ if (!repository || !lastRevision) {
this._repository = null;
- return Promise.resolve(null);
- }
-
- if (!to) {
- this._fetchingPromise = null;
this.enqueueToRender();
- return Promise.resolve(null);
+ return;
}
- var promise = CommitLog.fetchBetweenRevisions(repository, from || to, to);
+ let promise;
+ if (!precedingRevision || precedingRevision == lastRevision)
+ promise = CommitLog.fetchForSingleRevision(repository, lastRevision);
+ else
+ promise = CommitLog.fetchBetweenRevisions(repository, precedingRevision, lastRevision);
+ this._repository = repository;
this._fetchingPromise = promise;
- var self = this;
- var spinnerTimer = setTimeout(function () {
- self.enqueueToRender();
- }, 300);
-
- this._fetchingPromise.then(function (commits) {
- clearTimeout(spinnerTimer);
- if (self._fetchingPromise != promise)
+ this._fetchingPromise.then((commits) => {
+ if (this._fetchingPromise != promise)
return;
- self._fetchingPromise = null;
- self._commits = commits;
- }, function (error) {
- if (self._fetchingPromise != promise)
+ this._fetchingPromise = null;
+ this._commits = commits;
+ this.enqueueToRender();
+ }, (error) => {
+ if (this._fetchingPromise != promise)
return;
- self._fetchingPromise = null;
- self._commits = null;
+ this._fetchingPromise = null;
+ this._commits = null;
+ this.enqueueToRender();
});
- return this._fetchingPromise;
+ this.enqueueToRender();
}
render()
{
- var caption = '';
- if (this._repository && (this._commits || this._fetchingPromise))
- caption = this._repository.name();
- this.content().querySelector('caption').textContent = caption;
+ this.part('spinner').enqueueToRender();
+
+ const shouldShowRepositoryName = this._repository && (this._commits || this._fetchingPromise);
+ this.content('repository-name').textContent = shouldShowRepositoryName ? this._repository.name() : '';
+ this.content('spinner-container').style.display = this._fetchingPromise ? null : 'none';
+ this._renderCommitListLazily.evaluate(this._commits);
+ }
- var element = ComponentBase.createElement;
- var link = ComponentBase.createLink;
+ _renderCommitList(commits)
+ {
+ const element = ComponentBase.createElement;
+ const link = ComponentBase.createLink;
- this.renderReplace(this.content().querySelector('tbody'), (this._commits || []).map(function (commit) {
- var label = commit.label();
- var url = commit.url();
+ this.renderReplace(this.content('commits-list'), (commits || []).map((commit) => {
+ const label = commit.label();
+ const url = commit.url();
return element('tr', [
element('th', [element('h4', {class: 'revision'}, url ? link(label, commit.title(), url) : label), commit.author() || '']),
element('td', commit.message() ? commit.message().substring(0, 80) : '')]);
}));
-
- this.content().querySelector('.commits-viewer-spinner').style.display = this._fetchingPromise ? null : 'none';
}
static htmlTemplate()
{
return `
<div class="commits-viewer-container">
- <div class="commits-viewer-spinner"><spinner-icon></spinner-icon></div>
- <table class="commits-viewer-table">
- <caption></caption>
- <tbody>
- </tbody>
+ <div id="spinner-container"><spinner-icon id="spinner"></spinner-icon></div>
+ <table id="commits-viewer-table">
+ <caption id="repository-name"></caption>
+ <tbody id="commits-list"></tbody>
</table>
</div>
`;
overflow-y: scroll;
}
- .commits-viewer-table {
+ #commits-viewer-table {
width: 100%;
+ border-collapse: collapse;
+ border-bottom: solid 1px #ccc;
}
- .commits-viewer-table caption {
+ caption {
font-weight: inherit;
font-size: 1rem;
text-align: center;
padding: 0.2rem;
}
- .commits-viewer-table {
- border-collapse: collapse;
- border-bottom: solid 1px #ccc;
- }
-
- .commits-viewer-table .revision {
+ .revision {
white-space: nowrap;
font-weight: normal;
margin: 0;
padding: 0;
}
- .commits-viewer-table td,
- .commits-viewer-table th {
+ td, th {
word-break: break-word;
border-top: solid 1px #ccc;
padding: 0.2rem;
font-weight: normal;
}
- .commits-viewer-spinner {
+ #spinner-container {
margin-top: 2rem;
text-align: center;
}
if (this == previousCommit)
previousCommit = null;
- var repository = this._repository;
+ const repository = this._repository;
if (!previousCommit)
- return {from: null, to: this.revision(), repository: repository, label: this.label(), url: this.url()};
-
- var to = this.revision();
- var from = previousCommit.revision();
- var label = null;
- if (parseInt(to) == to) { // e.g. r12345.
- from = (parseInt(from) + 1).toString();
+ return {repository: repository, label: this.label(), url: this.url()};
+
+ const to = this.revision();
+ const from = previousCommit.revision();
+ let fromRevisionForURL = from;
+ let label = null;
+ if (parseInt(from) == from) { // e.g. r12345.
+ fromRevisionForURL = (parseInt(from) + 1).toString;
label = `r${from}-r${this.revision()}`;
- } else if (to.length == 40) { // e.g. git hash
+ } else if (to.length == 40) // e.g. git hash
label = `${from.substring(0, 8)}..${to.substring(0, 8)}`;
- } else
+ else
label = `${from} - ${to}`;
- return {from: from, to: to, repository: repository, label: label, url: repository.urlForRevisionRange(from, to)};
+ return {repository: repository, label: label, url: repository.urlForRevisionRange(from, to)};
}
- static fetchBetweenRevisions(repository, from, to)
+ static fetchBetweenRevisions(repository, precedingRevision, lastRevision)
{
- var params = [];
- if (from && to) {
- params.push(['from', from]);
- params.push(['to', to]);
- }
-
- var url = '../api/commits/' + repository.id() + '/?' + params.map(function (keyValue) {
- return encodeURIComponent(keyValue[0]) + '=' + encodeURIComponent(keyValue[1]);
- }).join('&');
-
-
- var cachedLogs = this._cachedCommitLogs(repository, from, to);
- if (cachedLogs)
- return new Promise(function (resolve) { resolve(cachedLogs); });
-
- var self = this;
- return RemoteAPI.getJSONWithStatus(url).then(function (data) {
- var commits = data['commits'].map(function (rawData) {
- rawData.repository = repository;
- return CommitLog.ensureSingleton(rawData.id, rawData);
- });
- self._cacheCommitLogs(repository, from, to, commits);
- return commits;
- });
+ // FIXME: The cache should be smarter about fetching a range within an already fetched range, etc...
+ // FIXME: We should evict some entires from the cache in cachedFetch.
+ return this.cachedFetch(`/api/commits/${repository.id()}/`, {precedingRevision, lastRevision})
+ .then((data) => this._constructFromRawData(repository, data));
}
- static _cachedCommitLogs(repository, from, to)
+ static fetchForSingleRevision(repository, revision)
{
- if (!this._caches)
- return null;
- var cache = this._caches[repository.id()];
- if (!cache)
- return null;
- // FIXME: Make each commit know of its parent, then we can do a better caching.
- return cache[from + '|' + to];
+ return this.cachedFetch(`/api/commits/${repository.id()}/${revision}`).then((data) => {
+ return this._constructFromRawData(repository, data);
+ });
}
- static _cacheCommitLogs(repository, from, to, logs)
+ static _constructFromRawData(repository, data)
{
- if (!this._caches)
- this._caches = {};
- if (!this._caches[repository.id()])
- this._caches[repository.id()] = {};
- this._caches[repository.id()][from + '|' + to] = logs;
+ return data['commits'].map((rawData) => {
+ rawData.repository = repository;
+ return CommitLog.ensureSingleton(rawData.id, rawData);
+ });
}
}
static cachedFetch(path, params, noCache)
{
- var query = [];
+ const query = [];
if (params) {
- for (var key in params)
- query.push(key + '=' + parseInt(params[key]));
+ for (let key in params)
+ query.push(key + '=' + escape(params[key]));
}
if (query.length)
path += '?' + query.join('&');
function onload() {
Instrumentation.endMeasuringTime('Remote', 'sendHttpRequest');
if (xhr.status != 200)
- return resject(xhr.status);
+ return reject(xhr.status);
resolve({statusCode: xhr.status, responseText: xhr.responseText});
};
});
- describe('/api/commits/<repository>/?from=<commit-1>&to=<commit-2>', () => {
+ describe('/api/commits/<repository>/?precedingRevision=<commit-1>&lastRevision=<commit-2>', () => {
it("should return RepositoryNotFound when there are no matching repository", () => {
return TestServer.remoteAPI().getJSON('/api/commits/WebKit/?from=210900&to=211000').then((response) => {
assert.equal(response['status'], 'RepositoryNotFound');
db.insert('repositories', {'id': 1, 'name': 'WebKit'}),
db.insert('commits', {'repository': 1, 'revision': '210950', 'time': '2017-01-20T03:49:37.887Z'})
]).then(() => {
- return TestServer.remoteAPI().getJSON('/api/commits/WebKit/?from=210900&to=211000');
+ return TestServer.remoteAPI().getJSON('/api/commits/WebKit/?precedingRevision=210900&lastRevision=211000');
}).then((response) => {
assert.equal(response['status'], 'UnknownCommit');
});
});
- it("should return an empty result when commits in the specified range have not reported", () => {
+ it("should return an empty result when commits in the specified range have not been reported", () => {
const db = TestServer.database();
return Promise.all([
db.insert('repositories', {'id': 1, 'name': 'WebKit'}),
db.insert('commits', {'repository': 1, 'revision': '210949', 'time': '2017-01-20T03:23:50.645Z'}),
db.insert('commits', {'repository': 1, 'revision': '210950', 'time': '2017-01-20T03:49:37.887Z'}),
]).then(() => {
- return TestServer.remoteAPI().getJSON('/api/commits/WebKit/?from=210949&to=210950');
+ return TestServer.remoteAPI().getJSON('/api/commits/WebKit/?precedingRevision=210949&lastRevision=210950');
+ }).then((response) => {
+ assert.equal(response['status'], 'OK');
+ assert.deepEqual(response['commits'], []);
+ });
+ });
+
+ it("should return InvalidCommitRange when the specified range is backwards", () => {
+ const db = TestServer.database();
+ return Promise.all([
+ db.insert('repositories', {'id': 1, 'name': 'WebKit'}),
+ db.insert('commits', {'repository': 1, 'revision': '210949', 'time': '2017-01-20T03:23:50.645Z'}),
+ db.insert('commits', {'repository': 1, 'revision': '210950', 'time': '2017-01-20T03:49:37.887Z'}),
+ ]).then(() => {
+ return TestServer.remoteAPI().getJSON('/api/commits/WebKit/?precedingRevision=210950&lastRevision=210949');
+ }).then((response) => {
+ assert.equal(response['status'], 'InvalidCommitRange');
+ });
+ });
+
+ it("should return use the commit order when time is not specified", () => {
+ const db = TestServer.database();
+ return Promise.all([
+ db.insert('repositories', {'id': 1, 'name': 'macOS'}),
+ db.insert('commits', {'repository': 1, 'revision': '10.12 16A323', order: 1, 'reported': true}),
+ db.insert('commits', {'repository': 1, 'revision': '10.12 16B2555', order: 2, 'reported': true}),
+ db.insert('commits', {'repository': 1, 'revision': '10.12 16B2657', order: 3, 'reported': true}),
+ ]).then(() => {
+ return TestServer.remoteAPI().getJSON('/api/commits/macOS/?precedingRevision=10.12%2016A323&lastRevision=10.12%2016B2657');
+ }).then((response) => {
+ assert.equal(response['status'], 'OK');
+ assert.deepEqual(response['commits'].map((commit) => commit['revision']), ['10.12 16B2555', '10.12 16B2657']);
+ });
+ });
+
+ it("should return InconsistentCommits when precedingRevision specifies a time but lastRevision does not", () => {
+ const db = TestServer.database();
+ return Promise.all([
+ db.insert('repositories', {'id': 1, 'name': 'macOS'}),
+ db.insert('commits', {'repository': 1, 'revision': '10.12 16A323', time: '2017-01-20T03:23:50.645Z', order: 1, 'reported': true}),
+ db.insert('commits', {'repository': 1, 'revision': '10.12 16B2555', order: 2, 'reported': true}),
+ db.insert('commits', {'repository': 1, 'revision': '10.12 16B2657', order: 3, 'reported': true}),
+ ]).then(() => {
+ return TestServer.remoteAPI().getJSON('/api/commits/macOS/?precedingRevision=10.12%2016A323&lastRevision=10.12%2016B2657');
+ }).then((response) => {
+ assert.equal(response['status'], 'InconsistentCommits');
+ });
+ });
+
+ it("should return InconsistentCommits when precedingRevision does not specify a time has a time but lastRevision does", () => {
+ const db = TestServer.database();
+ return Promise.all([
+ db.insert('repositories', {'id': 1, 'name': 'macOS'}),
+ db.insert('commits', {'repository': 1, 'revision': '10.12 16A323', order: 1, 'reported': true}),
+ db.insert('commits', {'repository': 1, 'revision': '10.12 16B2555', order: 2, 'reported': true}),
+ db.insert('commits', {'repository': 1, 'revision': '10.12 16B2657', time: '2017-01-20T03:23:50.645Z', order: 3, 'reported': true}),
+ ]).then(() => {
+ return TestServer.remoteAPI().getJSON('/api/commits/macOS/?precedingRevision=10.12%2016A323&lastRevision=10.12%2016B2657');
+ }).then((response) => {
+ assert.equal(response['status'], 'InconsistentCommits');
+ });
+ });
+
+ it("should return empty results when precedingRevision does not specify a time or an order has a time but lastRevision does", () => {
+ const db = TestServer.database();
+ return Promise.all([
+ db.insert('repositories', {'id': 1, 'name': 'macOS'}),
+ db.insert('commits', {'repository': 1, 'revision': '10.12 16A323', 'reported': true}),
+ db.insert('commits', {'repository': 1, 'revision': '10.12 16B2555', order: 2, 'reported': true}),
+ db.insert('commits', {'repository': 1, 'revision': '10.12 16B2657', order: 3, 'reported': true}),
+ ]).then(() => {
+ return TestServer.remoteAPI().getJSON('/api/commits/macOS/?precedingRevision=10.12%2016A323&lastRevision=10.12%2016B2657');
+ }).then((response) => {
+ assert.equal(response['status'], 'OK');
+ assert.deepEqual(response['commits'], []);
+ });
+ });
+
+ it("should return empty results when precedingRevision an order has a time but lastRevision does not", () => {
+ const db = TestServer.database();
+ return Promise.all([
+ db.insert('repositories', {'id': 1, 'name': 'macOS'}),
+ db.insert('commits', {'repository': 1, 'revision': '10.12 16A323', order: 1, 'reported': true}),
+ db.insert('commits', {'repository': 1, 'revision': '10.12 16B2555', order: 2, 'reported': true}),
+ db.insert('commits', {'repository': 1, 'revision': '10.12 16B2657', 'reported': true}),
+ ]).then(() => {
+ return TestServer.remoteAPI().getJSON('/api/commits/macOS/?precedingRevision=10.12%2016A323&lastRevision=10.12%2016B2657');
}).then((response) => {
assert.equal(response['status'], 'OK');
assert.deepEqual(response['commits'], []);
const db = TestServer.database();
return Promise.all([
db.insert('repositories', {'id': 1, 'name': 'WebKit'}),
+ db.insert('commits', {'repository': 1, 'revision': '210948', 'time': '2017-01-20T02:52:34.577Z', 'reported': true}),
db.insert('commits', {'repository': 1, 'revision': '210949', 'time': '2017-01-20T03:23:50.645Z', 'reported': true}),
db.insert('commits', {'repository': 1, 'revision': '210950', 'time': '2017-01-20T03:49:37.887Z', 'reported': true}),
]).then(() => {
- return TestServer.remoteAPI().getJSON('/api/commits/WebKit/?from=210949&to=210950');
+ return TestServer.remoteAPI().getJSON('/api/commits/WebKit/?precedingRevision=210948&lastRevision=210950');
}).then((result) => {
assert.equal(result['status'], 'OK');
assert.deepEqual(result['commits'].length, 2);
});
it("should not include a revision not within the specified range", () => {
+ const db = TestServer.database();
const remote = TestServer.remoteAPI();
- return addSlaveForReport(subversionCommits).then(() => {
+ return Promise.all([
+ db.insert('repositories', {'id': 1, 'name': 'WebKit'}),
+ db.insert('commits', {'repository': 1, 'revision': '210947', 'time': '2017-01-20T02:38:45.485Z', 'reported': false}),
+ db.insert('commits', {'repository': 1, 'revision': '210948', 'time': '2017-01-20T02:52:34.577Z', 'reported': false}),
+ db.insert('commits', {'repository': 1, 'revision': '210949', 'time': '2017-01-20T03:23:50.645Z', 'reported': false}),
+ db.insert('commits', {'repository': 1, 'revision': '210950', 'time': '2017-01-20T03:49:37.887Z', 'reported': false}),
+ ]).then(() => {
+ return addSlaveForReport(subversionCommits);
+ }).then(() => {
return remote.postJSONWithStatus('/api/report-commits/', subversionCommits);
}).then(() => {
- return remote.getJSON('/api/commits/WebKit/?from=210948&to=210949');
+ return remote.getJSON('/api/commits/WebKit/?precedingRevision=210947&lastRevision=210949');
}).then((result) => {
assert.equal(result['status'], 'OK');
assert.deepEqual(result['commits'].length, 2);
describe('diff', function () {
it('should use label() as the label the previous commit is missing', function () {
assert.deepEqual(webkitCommit().diff(), {
- from: null,
- to: '200805',
label: 'r200805',
url: 'http://trac.webkit.org/changeset/200805',
repository: MockModels.webkit
});
assert.deepEqual(gitWebKitCommit().diff(), {
- from: null,
- to: '6f8b0dbbda95a440503b88db1dd03dad3a7b07fb',
label: '6f8b0dbb',
url: 'http://trac.webkit.org/changeset/6f8b0dbbda95a440503b88db1dd03dad3a7b07fb',
repository: MockModels.webkit,
});
assert.deepEqual(osxCommit().diff(), {
- from: null,
- to: '10.11.4 15E65',
label: '10.11.4 15E65',
url: '',
repository: MockModels.osx,
it('should use increment the old SVN revision by 1', function () {
assert.deepEqual(webkitCommit().diff(oldWebKitCommit()), {
- from: '200575',
- to: '200805',
- label: 'r200575-r200805',
+ label: 'r200574-r200805',
url: '',
repository: MockModels.webkit
});
it('should truncate a Git hash at 8th character', function () {
assert.deepEqual(gitWebKitCommit().diff(oldGitWebKitCommit()), {
- from: 'ffda14e6db0746d10d0f050907e4a7325851e502',
- to: '6f8b0dbbda95a440503b88db1dd03dad3a7b07fb',
label: 'ffda14e6..6f8b0dbb',
url: '',
repository: MockModels.webkit
it('should surround "-" with spaces', function () {
assert.deepEqual(osxCommit().diff(oldOSXCommit()), {
- from: '10.11.3 15D21',
- to: '10.11.4 15E65',
label: '10.11.3 15D21 - 10.11.4 15E65',
url: '',
repository: MockModels.osx
git://git.webkit.org / WebKit-https.git / commitdiff