Log Message
Fix the bug that 'TestGroupResultsViewer' creates unnecessary rows. https://bugs.webkit.org/show_bug.cgi?id=181967
Reviewed by Ryosuke Niwa. Fixed a bug caused by a typo in CommitSet.equals, which makes it returns incorrect results for most comparison between a CommitSet and a MeasurementCommitSet. MeasurementCommitSet does not have full information for the commits, thus, it cannot build mappings between root/patch/owner commit/requires build to repository. When querying whether a given repository needs to be built, MeasurementCommitSet will return undefined. Due to 'undefined != false', this equality check will fail. Making 'CommitSet.requiresBuildForRepository' defaults to 'false' would fix this bug. * public/v3/models/commit-set.js: (CommitSet.prototype.requiresBuildForRepository): Make it return false when key does not exist instead of 'undefined'. (CommitSet.prototype.equals): Fixed the typo that causes the bug. Use wrapped functions instead of querying the mapping directly. * unit-tests/commit-set-tests.js: Added unit tests.
Modified Paths
Diff
Modified: trunk/Websites/perf.webkit.org/ChangeLog (227567 => 227568)
--- trunk/Websites/perf.webkit.org/ChangeLog 2018-01-24 22:11:19 UTC (rev 227567)
+++ trunk/Websites/perf.webkit.org/ChangeLog 2018-01-24 22:17:33 UTC (rev 227568)
@@ -1,3 +1,26 @@
+2018-01-22 Dewei Zhu <[email protected]>
+
+ Fix the bug that 'TestGroupResultsViewer' creates unnecessary rows.
+ https://bugs.webkit.org/show_bug.cgi?id=181967
+
+ Reviewed by Ryosuke Niwa.
+
+ Fixed a bug caused by a typo in CommitSet.equals, which makes it returns incorrect results for most
+ comparison between a CommitSet and a MeasurementCommitSet.
+
+ MeasurementCommitSet does not have full information for the commits, thus, it cannot build mappings
+ between root/patch/owner commit/requires build to repository. When querying whether a given repository
+ needs to be built, MeasurementCommitSet will return undefined. Due to 'undefined != false', this
+ equality check will fail. Making 'CommitSet.requiresBuildForRepository' defaults to 'false' would fix
+ this bug.
+
+ * public/v3/models/commit-set.js:
+ (CommitSet.prototype.requiresBuildForRepository): Make it return false when key does not exist
+ instead of 'undefined'.
+ (CommitSet.prototype.equals): Fixed the typo that causes the bug.
+ Use wrapped functions instead of querying the mapping directly.
+ * unit-tests/commit-set-tests.js: Added unit tests.
+
2018-01-18 Dewei Zhu <[email protected]>
'run-test.py' script should make sure 'node_modules' directory exists before installing node packages.
Modified: trunk/Websites/perf.webkit.org/public/v3/models/commit-set.js (227567 => 227568)
--- trunk/Websites/perf.webkit.org/public/v3/models/commit-set.js 2018-01-24 22:11:19 UTC (rev 227567)
+++ trunk/Websites/perf.webkit.org/public/v3/models/commit-set.js 2018-01-24 22:17:33 UTC (rev 227568)
@@ -87,7 +87,7 @@
patchForRepository(repository) { return this._repositoryToPatchMap.get(repository); }
rootForRepository(repository) { return this._repositoryToRootMap.get(repository); }
- requiresBuildForRepository(repository) { return this._repositoryRequiresBuildMap.get(repository); }
+ requiresBuildForRepository(repository) { return this._repositoryRequiresBuildMap.get(repository) || false; }
// FIXME: This should return a Date object.
latestCommitTime()
@@ -108,13 +108,13 @@
for (const [repository, commit] of this._repositoryToCommitMap) {
if (commit != other._repositoryToCommitMap.get(repository))
return false;
- if (this._repositoryToPatchMap.get(repository) != other._repositoryToPatchMap.get(repository))
+ if (this.patchForRepository(repository) != other.patchForRepository(repository))
return false;
- if (this._repositoryToRootMap.get(repository) != other._repositoryToRootMap.get(repository))
+ if (this.rootForRepository(repository) != other.rootForRepository(repository))
return false;
- if (this._repositoryToCommitOwnerMap.get(repository) != other._repositoryToCommitMap.get(repository))
+ if (this.ownerCommitForRepository(repository) != other.ownerCommitForRepository(repository))
return false;
- if (this._repositoryRequiresBuildMap.get(repository) != other._repositoryRequiresBuildMap.get(repository))
+ if (this.requiresBuildForRepository(repository) != other.requiresBuildForRepository(repository))
return false;
}
return CommitSet.areCustomRootsEqual(this._customRoots, other._customRoots);
Modified: trunk/Websites/perf.webkit.org/unit-tests/commit-set-tests.js (227567 => 227568)
--- trunk/Websites/perf.webkit.org/unit-tests/commit-set-tests.js 2018-01-24 22:11:19 UTC (rev 227567)
+++ trunk/Websites/perf.webkit.org/unit-tests/commit-set-tests.js 2018-01-24 22:17:33 UTC (rev 227568)
@@ -7,16 +7,28 @@
function createPatch()
{
- return new UploadedFile(453, {'createdAt': new Date('2017-05-01T19:16:53Z'), 'filename': 'patch.dat', 'extension': '.dat', 'author': 'some user',
+ return UploadedFile.ensureSingleton(453, {'createdAt': new Date('2017-05-01T19:16:53Z'), 'filename': 'patch.dat', 'extension': '.dat', 'author': 'some user',
size: 534637, sha256: '169463c8125e07c577110fe144ecd63942eb9472d438fc0014f474245e5df8a1'});
}
+function createAnotherPatch()
+{
+ return UploadedFile.ensureSingleton(454, {'createdAt': new Date('2017-05-01T19:16:53Z'), 'filename': 'patch.dat', 'extension': '.dat', 'author': 'some user',
+ size: 534611, sha256: '169463c8125e07c577110fe144ecd63942eb9472d438fc0014f474245e5dfaaa'});
+}
+
function createRoot()
{
- return new UploadedFile(456, {'createdAt': new Date('2017-05-01T21:03:27Z'), 'filename': 'root.dat', 'extension': '.dat', 'author': 'some user',
+ return UploadedFile.ensureSingleton(456, {'createdAt': new Date('2017-05-01T21:03:27Z'), 'filename': 'root.dat', 'extension': '.dat', 'author': 'some user',
size: 16452234, sha256: '03eed7a8494ab8794c44b7d4308e55448fc56f4d6c175809ba968f78f656d58d'});
}
+function createAnotherRoot()
+{
+ return UploadedFile.ensureSingleton(457, {'createdAt': new Date('2017-05-01T21:03:27Z'), 'filename': 'root.dat', 'extension': '.dat', 'author': 'some user',
+ size: 16452111, sha256: '03eed7a8494ab8794c44b7d4308e55448fc56f4d6c175809ba968f78f656dbbb'});
+}
+
function customCommitSetWithoutOwnedCommit()
{
const customCommitSet = new CustomCommitSet;
@@ -54,7 +66,7 @@
function ownerCommit()
{
- return new CommitLog(5, {
+ return CommitLog.ensureSingleton(5, {
repository: MockModels.ownerRepository,
revision: 'owner-commit-0',
ownsCommits: true,
@@ -64,7 +76,7 @@
function partialOwnerCommit()
{
- return new CommitLog(5, {
+ return CommitLog.ensureSingleton(5, {
repository: MockModels.ownerRepository,
revision: 'owner-commit-0',
ownsCommits: null,
@@ -84,7 +96,7 @@
function webkitCommit()
{
- return new CommitLog(2017, {
+ return CommitLog.ensureSingleton(2017, {
repository: MockModels.webkit,
revision: 'webkit-commit-0',
ownsCommits: false,
@@ -92,6 +104,103 @@
});
}
+describe('CommitSet', () => {
+ MockRemoteAPI.inject();
+ MockModels.inject();
+
+ function oneCommitSet()
+ {
+ return CommitSet.ensureSingleton(1, {
+ revisionItems: [{ commit: webkitCommit(), requiresBuild: false }],
+ customRoots: []
+ });
+ }
+
+ function anotherCommitSet()
+ {
+ return CommitSet.ensureSingleton(2, {
+ revisionItems: [{ commit: webkitCommit(), requiresBuild: false }],
+ customRoots: []
+ });
+ }
+
+ function commitSetWithPatch()
+ {
+ return CommitSet.ensureSingleton(3, {
+ revisionItems: [{ commit: webkitCommit(), requiresBuild: false, patch: createPatch() }],
+ customRoots: []
+ });
+ }
+
+ function commitSetWithAnotherPatch()
+ {
+ return CommitSet.ensureSingleton(4, {
+ revisionItems: [{ commit: webkitCommit(), requiresBuild: false, patch: createAnotherPatch() }],
+ customRoots: []
+ });
+ }
+
+ function commitSetWithRoot()
+ {
+ return CommitSet.ensureSingleton(5, {
+ revisionItems: [{ commit: webkitCommit(), requiresBuild: false }],
+ customRoots: [createRoot()]
+ });
+ }
+
+ function anotherCommitSetWithRoot()
+ {
+ return CommitSet.ensureSingleton(6, {
+ revisionItems: [{ commit: webkitCommit(), requiresBuild: false }],
+ customRoots: [createAnotherRoot()]
+ });
+ }
+
+ function oneMeasurementCommitSet()
+ {
+ return MeasurementCommitSet.ensureSingleton(1, [
+ [2017, 11, 'webkit-commit-0', 1456932773000]
+ ]);
+ }
+
+ describe('equals', () => {
+ it('should return false if patches for same repository are different', () => {
+ assert(!commitSetWithPatch().equals(commitSetWithAnotherPatch()));
+ assert(!commitSetWithAnotherPatch().equals(commitSetWithPatch()));
+ });
+
+ it('should return false if patch is only specified in one commit set', () => {
+ assert(!oneCommitSet().equals(commitSetWithPatch()));
+ assert(!commitSetWithPatch().equals(oneCommitSet()));
+ });
+
+ it('should return false if roots for same repository are different', () => {
+ assert(!commitSetWithRoot().equals(anotherCommitSetWithRoot()));
+ assert(!anotherCommitSetWithRoot().equals(commitSetWithRoot()));
+ });
+
+ it('should return false if root is only specified in one commit set', () => {
+ assert(!commitSetWithRoot().equals(oneCommitSet()));
+ assert(!oneCommitSet().equals(commitSetWithRoot()));
+ });
+
+ it('should return true when comparing two identical commit set', () => {
+ assert(oneCommitSet().equals(oneCommitSet()));
+ assert(anotherCommitSet().equals(anotherCommitSet()));
+ assert(commitSetWithPatch().equals(commitSetWithPatch()));
+ assert(commitSetWithAnotherPatch().equals(commitSetWithAnotherPatch()));
+ assert(oneMeasurementCommitSet().equals(oneMeasurementCommitSet()));
+ assert(commitSetWithRoot().equals(commitSetWithRoot()));
+ assert(anotherCommitSetWithRoot().equals(anotherCommitSetWithRoot()));
+ });
+
+ it('should be able to compare between CommitSet and MeasurementCommitSet', () => {
+ assert(oneCommitSet().equals(oneMeasurementCommitSet()));
+ assert(oneMeasurementCommitSet().equals(oneCommitSet()));
+ });
+ })
+});
+
describe('IntermediateCommitSet', () => {
MockRemoteAPI.inject();
MockModels.inject();
_______________________________________________ webkit-changes mailing list [email protected] https://lists.webkit.org/mailman/listinfo/webkit-changes
