Log Message
Added 'CommitSet.diff' which will be shared between multiple independent incoming changes. https://bugs.webkit.org/show_bug.cgi?id=184368
Reviewed by Ryosuke Niwa. 'CommitSet.diff' will be used in multiple independent incoming changes. It would be easier to make this a separate change to parallelize the changes depends on this API. * public/v3/models/commit-set.js: (CommitSet.prototype.createNameWithoutCollision): Moved from 'AnalysisTaskPage' and make it more generic. (CommitSet.prototype.diff): Describe differences between 2 commit sets including commit, root and patch differences. * public/v3/pages/analysis-task-page.js: Move 'AnalysisTaskPage._createRetryNameForTestGroup' to CommitSet in a more generic form. (AnalysisTaskPage.prototype._retryCurrentTestGroup): Use 'CommitSet.withoutRootPatchOrOwnedCommit' instead. (AnalysisTaskPage.prototype._createRetryNameForTestGroup): Moved to CommitSet in a more generic form. * unit-tests/commit-set-tests.js: Added unit tests for 'CommitSet.diff'.
Modified Paths
Diff
Modified: trunk/Websites/perf.webkit.org/ChangeLog (230440 => 230441)
--- trunk/Websites/perf.webkit.org/ChangeLog 2018-04-09 16:10:52 UTC (rev 230440)
+++ trunk/Websites/perf.webkit.org/ChangeLog 2018-04-09 16:27:52 UTC (rev 230441)
@@ -1,3 +1,21 @@
+2018-04-06 Dewei Zhu <dewei_...@apple.com>
+
+ Added 'CommitSet.diff' which will be shared between multiple independent incoming changes.
+ https://bugs.webkit.org/show_bug.cgi?id=184368
+
+ Reviewed by Ryosuke Niwa.
+
+ 'CommitSet.diff' will be used in multiple independent incoming changes.
+ It would be easier to make this a separate change to parallelize the changes depends on this API.
+
+ * public/v3/models/commit-set.js:
+ (CommitSet.prototype.createNameWithoutCollision): Moved from 'AnalysisTaskPage' and make it more generic.
+ (CommitSet.prototype.diff): Describe differences between 2 commit sets including commit, root and patch differences.
+ * public/v3/pages/analysis-task-page.js: Move 'AnalysisTaskPage._createRetryNameForTestGroup' to CommitSet in a more generic form.
+ (AnalysisTaskPage.prototype._retryCurrentTestGroup): Use 'CommitSet.withoutRootPatchOrOwnedCommit' instead.
+ (AnalysisTaskPage.prototype._createRetryNameForTestGroup): Moved to CommitSet in a more generic form.
+ * unit-tests/commit-set-tests.js: Added unit tests for 'CommitSet.diff'.
+
2018-04-05 Dewei Zhu <dewei_...@apple.com>
Fix a bug introduced in r230295 that A/B test result is not shown.
Modified: trunk/Websites/perf.webkit.org/public/v3/models/commit-set.js (230440 => 230441)
--- trunk/Websites/perf.webkit.org/public/v3/models/commit-set.js 2018-04-09 16:10:52 UTC (rev 230440)
+++ trunk/Websites/perf.webkit.org/public/v3/models/commit-set.js 2018-04-09 16:27:52 UTC (rev 230441)
@@ -145,6 +145,82 @@
}
return false;
}
+
+ static createNameWithoutCollision(name, existingNameSet)
+ {
+ console.assert(existingNameSet instanceof Set);
+ if (!existingNameSet.has(name))
+ return name;
+ const nameWithNumberMatch = name.match(/(.+?)\s*\(\s*(\d+)\s*\)\s*$/);
+ let number = 1;
+ if (nameWithNumberMatch) {
+ name = nameWithNumberMatch[1];
+ number = parseInt(nameWithNumberMatch[2]);
+ }
+
+ let newName;
+ do {
+ number++;
+ newName = `${name} (${number})`;
+ } while (existingNameSet.has(newName));
+
+ return newName;
+ }
+
+ static diff(firstCommitSet, secondCommitSet)
+ {
+ console.assert(!firstCommitSet.equals(secondCommitSet));
+ const allRepositories = new Set([...firstCommitSet.repositories(), ...secondCommitSet.repositories()]);
+ const sortedRepositories = Repository.sortByNamePreferringOnesWithURL([...allRepositories]);
+ const nameParts = [];
+ const missingCommit = {label: () => 'none'};
+ const missingPatch = {filename: () => 'none'};
+ const makeNameGenerator = () => {
+ const existingNameSet = new Set;
+ return (name) => {
+ const newName = CommitSet.createNameWithoutCollision(name, existingNameSet);
+ existingNameSet.add(newName);
+ return newName;
+ }
+ };
+
+ for (const repository of sortedRepositories) {
+ const firstCommit = firstCommitSet.commitForRepository(repository) || missingCommit;
+ const secondCommit = secondCommitSet.commitForRepository(repository) || missingCommit;
+ const firstPatch = firstCommitSet.patchForRepository(repository) || missingPatch;
+ const secondPatch = secondCommitSet.patchForRepository(repository) || missingPatch;
+ const nameGenerator = makeNameGenerator();
+
+ if (firstCommit == secondCommit && firstPatch == secondPatch)
+ continue;
+
+ if (firstCommit != secondCommit && firstPatch == secondPatch)
+ nameParts.push(`${repository.name()}: ${secondCommit.diff(firstCommit).label}`);
+
+ // FIXME: It would be nice if we can abbreviate the name when it's too long.
+ const nameForFirstPatch = nameGenerator(firstPatch.filename());
+ const nameForSecondPath = nameGenerator(secondPatch.filename());
+
+ if (firstCommit == secondCommit && firstPatch != secondPatch)
+ nameParts.push(`${repository.name()}: ${nameForFirstPatch} - ${nameForSecondPath}`);
+
+ if (firstCommit != secondCommit && firstPatch != secondPatch)
+ nameParts.push(`${repository.name()}: ${firstCommit.label()} with ${nameForFirstPatch} - ${secondCommit.label()} with ${nameForSecondPath}`);
+ }
+
+ if (firstCommitSet.allRootFiles().length || secondCommitSet.allRootFiles().length) {
+ const firstRootFileSet = new Set(firstCommitSet.allRootFiles());
+ const secondRootFileSet = new Set(secondCommitSet.allRootFiles());
+ const uniqueInFirstCommitSet = firstCommitSet.allRootFiles().filter((rootFile) => !secondRootFileSet.has(rootFile));
+ const uniqueInSecondCommitSet = secondCommitSet.allRootFiles().filter((rootFile) => !firstRootFileSet.has(rootFile));
+ const nameGenerator = makeNameGenerator();
+ const firstDescription = uniqueInFirstCommitSet.map((rootFile) => nameGenerator(rootFile.filename())).join(', ');
+ const secondDescription = uniqueInSecondCommitSet.map((rootFile) => nameGenerator(rootFile.filename())).join(', ');
+ nameParts.push(`Roots: ${firstDescription || 'none'} - ${secondDescription || 'none'}`);
+ }
+
+ return nameParts.join(' ');
+ }
}
class MeasurementCommitSet extends CommitSet {
Modified: trunk/Websites/perf.webkit.org/public/v3/pages/analysis-task-page.js (230440 => 230441)
--- trunk/Websites/perf.webkit.org/public/v3/pages/analysis-task-page.js 2018-04-09 16:10:52 UTC (rev 230440)
+++ trunk/Websites/perf.webkit.org/public/v3/pages/analysis-task-page.js 2018-04-09 16:27:52 UTC (rev 230441)
@@ -785,7 +785,8 @@
_retryCurrentTestGroup(testGroup, repetitionCount)
{
- const newName = this._createRetryNameForTestGroup(testGroup.name());
+ const existingNames = (this._testGroups || []).map((group) => group.name());
+ const newName = CommitSet.createNameWithoutCollision(testGroup.name(), new Set(existingNames));
const commitSetList = testGroup.requestedCommitSets();
const platform = this._task.platform() || testGroup.platform();
return TestGroup.createWithCustomConfiguration(this._task, platform, testGroup.test(), newName, repetitionCount, commitSetList)
@@ -841,24 +842,6 @@
});
}
- _createRetryNameForTestGroup(name)
- {
- var nameWithNumberMatch = name.match(/(.+?)\s*\(\s*(\d+)\s*\)\s*$/);
- var number = 1;
- if (nameWithNumberMatch) {
- name = nameWithNumberMatch[1];
- number = parseInt(nameWithNumberMatch[2]);
- }
-
- var newName;
- do {
- number++;
- newName = `${name} (${number})`;
- } while (this._hasDuplicateTestGroupName(newName));
-
- return newName;
- }
-
_hasDuplicateTestGroupName(name)
{
console.assert(this._testGroups);
Modified: trunk/Websites/perf.webkit.org/unit-tests/commit-set-tests.js (230440 => 230441)
--- trunk/Websites/perf.webkit.org/unit-tests/commit-set-tests.js 2018-04-09 16:10:52 UTC (rev 230440)
+++ trunk/Websites/perf.webkit.org/unit-tests/commit-set-tests.js 2018-04-09 16:27:52 UTC (rev 230441)
@@ -29,6 +29,12 @@
size: 16452111, sha256: '03eed7a8494ab8794c44b7d4308e55448fc56f4d6c175809ba968f78f656dbbb'});
}
+function createSharedRoot()
+{
+ return UploadedFile.ensureSingleton(458, {'createdAt': new Date('2017-05-01T22:03:27Z'), 'filename': 'root.dat', 'extension': '.dat', 'author': 'some user',
+ size: 16452111, sha256: '03eed7a8494ab8794c44b7d4308e55448fc56f4aac175809ba968f78f656dbbb'});
+}
+
function customCommitSetWithoutOwnedCommit()
{
const customCommitSet = new CustomCommitSet;
@@ -104,6 +110,56 @@
});
}
+function anotherWebKitCommit()
+{
+ return CommitLog.ensureSingleton(2018, {
+ repository: MockModels.webkit,
+ revision: 'webkit-commit-1',
+ ownsCommits: false,
+ time: 1456932773000
+ });
+}
+
+function commitWithSVNRevision()
+{
+ return CommitLog.ensureSingleton(2019, {
+ repository: MockModels.webkit,
+ revision: '12345',
+ ownsCommits: false,
+ time: 1456932773000
+ });
+}
+
+function anotherCommitWithSVNRevision()
+{
+ return CommitLog.ensureSingleton(2020, {
+ repository: MockModels.webkit,
+ revision: '45678',
+ ownsCommits: false,
+ time: 1456932773000
+ });
+}
+
+function commitWithGitRevision()
+{
+ return CommitLog.ensureSingleton(2021, {
+ repository: MockModels.webkitGit,
+ revision: '13a0590d34f26fda3953c42ff833132a1a6f6f5a',
+ ownsCommits: false,
+ time: 1456932773000
+ });
+}
+
+function anotherCommitWithGitRevision()
+{
+ return CommitLog.ensureSingleton(2022, {
+ repository: MockModels.webkitGit,
+ revision: '2f8dd3321d4f51c04f4e2019428ce9ffe97f1ef1',
+ ownsCommits: false,
+ time: 1456932773000
+ });
+}
+
describe('CommitSet', () => {
MockRemoteAPI.inject();
MockModels.inject();
@@ -156,6 +212,78 @@
});
}
+ function commitSetWithTwoRoots()
+ {
+ return CommitSet.ensureSingleton(7, {
+ revisionItems: [{ commit: webkitCommit(), requiresBuild: false }],
+ customRoots: [createRoot(), createSharedRoot()]
+ });
+ }
+
+ function commitSetWithAnotherWebKitCommit()
+ {
+ return CommitSet.ensureSingleton(8, {
+ revisionItems: [{ commit: anotherWebKitCommit(), requiresBuild: false }],
+ customRoots: []
+ });
+ }
+
+ function commitSetWithAnotherCommitPatchAndRoot()
+ {
+ return CommitSet.ensureSingleton(9, {
+ revisionItems: [{ commit: anotherWebKitCommit(), requiresBuild: true, patch: createPatch()}],
+ customRoots: [createRoot(), createSharedRoot()]
+ });
+ }
+
+ function commitSetWithSVNCommit()
+ {
+ return CommitSet.ensureSingleton(10, {
+ revisionItems: [{ commit: commitWithSVNRevision(), requiresBuild: false }],
+ customRoots: []
+ });
+ }
+
+ function anotherCommitSetWithSVNCommit()
+ {
+ return CommitSet.ensureSingleton(11, {
+ revisionItems: [{ commit: anotherCommitWithSVNRevision(), requiresBuild: false }],
+ customRoots: []
+ });
+ }
+
+ function commitSetWithGitCommit()
+ {
+ return CommitSet.ensureSingleton(12, {
+ revisionItems: [{ commit: commitWithGitRevision(), requiresBuild: false }],
+ customRoots: []
+ });
+ }
+
+ function anotherCommitSetWithGitCommit()
+ {
+ return CommitSet.ensureSingleton(13, {
+ revisionItems: [{ commit: anotherCommitWithGitRevision(), requiresBuild: false }],
+ customRoots: []
+ });
+ }
+
+ function commitSetWithTwoCommits()
+ {
+ return CommitSet.ensureSingleton(14, {
+ revisionItems: [{ commit: commitWithGitRevision(), requiresBuild: false }, { commit: commitWithSVNRevision(), requiresBuild: false }],
+ customRoots: []
+ });
+ }
+
+ function anotherCommitSetWithTwoCommits()
+ {
+ return CommitSet.ensureSingleton(15, {
+ revisionItems: [{ commit: anotherCommitWithGitRevision(), requiresBuild: false }, { commit: anotherCommitWithSVNRevision(), requiresBuild: false }],
+ customRoots: []
+ });
+ }
+
function oneMeasurementCommitSet()
{
return MeasurementCommitSet.ensureSingleton(1, [
@@ -198,7 +326,30 @@
assert(oneCommitSet().equals(oneMeasurementCommitSet()));
assert(oneMeasurementCommitSet().equals(oneCommitSet()));
});
- })
+ });
+
+ describe('diff', () => {
+ it('should describe patch difference', () => {
+ assert.equal(CommitSet.diff(commitSetWithPatch(), commitSetWithAnotherPatch()), 'WebKit: patch.dat - patch.dat (2)');
+ });
+
+ it('should describe root difference', () => {
+ assert.equal(CommitSet.diff(commitSetWithRoot(), anotherCommitSetWithRoot()), 'Roots: root.dat - root.dat (2)');
+ assert.equal(CommitSet.diff(commitSetWithRoot(), commitSetWithTwoRoots()), 'Roots: none - root.dat');
+ assert.equal(CommitSet.diff(commitSetWithTwoRoots(), oneCommitSet()), 'Roots: root.dat, root.dat (2) - none');
+ });
+
+ it('should describe commit difference', () => {
+ assert.equal(CommitSet.diff(oneCommitSet(), commitSetWithAnotherWebKitCommit()), 'WebKit: webkit-commit-0 - webkit-commit-1');
+ assert.equal(CommitSet.diff(commitSetWithSVNCommit(), anotherCommitSetWithSVNCommit()), 'WebKit: r12345-r45678');
+ assert.equal(CommitSet.diff(commitSetWithGitCommit(), anotherCommitSetWithGitCommit()), 'WebKit-Git: 13a0590d..2f8dd332');
+ assert.equal(CommitSet.diff(commitSetWithTwoCommits(), anotherCommitSetWithTwoCommits()), 'WebKit: r12345-r45678 WebKit-Git: 13a0590d..2f8dd332');
+ });
+
+ it('should describe commit root and patch difference', () => {
+ assert.equal(CommitSet.diff(oneCommitSet(), commitSetWithAnotherCommitPatchAndRoot()), 'WebKit: webkit-commit-0 with none - webkit-commit-1 with patch.dat Roots: none - root.dat, root.dat (2)');
+ });
+ });
});
describe('IntermediateCommitSet', () => {
_______________________________________________ webkit-changes mailing list webkit-changes@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-changes
