Add check for binary in total calculations

In the calculations for aggregating size_delta_inserted and
size_delta_deleted, which aggregate the total binary additions and
deletions, there was a missing check for whether a file was binary or
not before adding the value. This caused a strange total binary change
to appear. This change adds the binary check and also updates tests
to accurately represent that non-binary changes can have size_delta as
an attribute.

Bug: Issue 4992
Change-Id: I52da1c8f9fdcf6be94658f1b95721453a71a4213
This commit is contained in:
Becky Siegel
2016-11-22 15:55:14 -08:00
parent 7f95f255c1
commit 2e30833c4d
2 changed files with 35 additions and 9 deletions

View File

@@ -179,8 +179,10 @@
var inserted = obj.lines_inserted ? obj.lines_inserted : 0; var inserted = obj.lines_inserted ? obj.lines_inserted : 0;
var deleted = obj.lines_deleted ? obj.lines_deleted : 0; var deleted = obj.lines_deleted ? obj.lines_deleted : 0;
var total_size = (obj.size && obj.binary) ? obj.size : 0; var total_size = (obj.size && obj.binary) ? obj.size : 0;
var size_delta_inserted = obj.size_delta > 0 ? obj.size_delta : 0; var size_delta_inserted =
var size_delta_deleted = obj.size_delta < 0 ? obj.size_delta : 0; obj.binary && obj.size_delta > 0 ? obj.size_delta : 0;
var size_delta_deleted =
obj.binary && obj.size_delta < 0 ? obj.size_delta : 0;
return { return {
inserted: acc.inserted + inserted, inserted: acc.inserted + inserted,

View File

@@ -103,8 +103,20 @@ limitations under the License.
test('calculate totals for patch number', function() { test('calculate totals for patch number', function() {
element._files = [ element._files = [
{__path: '/COMMIT_MSG', lines_inserted: 9}, {__path: '/COMMIT_MSG', lines_inserted: 9},
{__path: 'file_added_in_rev2.txt', lines_inserted: 1, lines_deleted: 1}, {
{__path: 'myfile.txt', lines_inserted: 1, lines_deleted: 1}, __path: 'file_added_in_rev2.txt',
lines_inserted: 1,
lines_deleted: 1,
size_delta: 10,
size: 100,
},
{
__path: 'myfile.txt',
lines_inserted: 1,
lines_deleted: 1,
size_delta: 10,
size: 100,
},
]; ];
assert.deepEqual(element._patchChange, { assert.deepEqual(element._patchChange, {
inserted: 2, inserted: 2,
@@ -113,6 +125,8 @@ limitations under the License.
size_delta_deleted: 0, size_delta_deleted: 0,
total_size: 0, total_size: 0,
}); });
assert.isTrue(element._hideBinaryChangeTotals);
assert.isFalse(element._hideChangeTotals);
// Test with a commit message that isn't the first file. // Test with a commit message that isn't the first file.
element._files = [ element._files = [
@@ -127,6 +141,8 @@ limitations under the License.
size_delta_deleted: 0, size_delta_deleted: 0,
total_size: 0, total_size: 0,
}); });
assert.isTrue(element._hideBinaryChangeTotals);
assert.isFalse(element._hideChangeTotals);
// Test with no commit message. // Test with no commit message.
element._files = [ element._files = [
@@ -140,6 +156,8 @@ limitations under the License.
size_delta_deleted: 0, size_delta_deleted: 0,
total_size: 0, total_size: 0,
}); });
assert.isTrue(element._hideBinaryChangeTotals);
assert.isFalse(element._hideChangeTotals);
// Test with files missing either lines_inserted or lines_deleted. // Test with files missing either lines_inserted or lines_deleted.
element._files = [ element._files = [
@@ -153,6 +171,8 @@ limitations under the License.
size_delta_deleted: 0, size_delta_deleted: 0,
total_size: 0, total_size: 0,
}); });
assert.isTrue(element._hideBinaryChangeTotals);
assert.isFalse(element._hideChangeTotals);
}); });
test('binary only files', function() { test('binary only files', function() {
@@ -168,6 +188,8 @@ limitations under the License.
size_delta_deleted: -5, size_delta_deleted: -5,
total_size: 220, total_size: 220,
}); });
assert.isFalse(element._hideBinaryChangeTotals);
assert.isTrue(element._hideChangeTotals);
}); });
test('binary and regular files', function() { test('binary and regular files', function() {
@@ -175,7 +197,7 @@ limitations under the License.
{__path: '/COMMIT_MSG', lines_inserted: 9}, {__path: '/COMMIT_MSG', lines_inserted: 9},
{__path: 'file_binary', binary: true, size_delta: 10, size: 100}, {__path: 'file_binary', binary: true, size_delta: 10, size: 100},
{__path: 'file_binary', binary: true, size_delta: -5, size: 120}, {__path: 'file_binary', binary: true, size_delta: -5, size: 120},
{__path: 'myfile.txt', lines_deleted: 5}, {__path: 'myfile.txt', lines_deleted: 5, size_delta: -10, size: 100},
{__path: 'myfile2.txt', lines_inserted: 10}, {__path: 'myfile2.txt', lines_inserted: 10},
]; ];
assert.deepEqual(element._patchChange, { assert.deepEqual(element._patchChange, {
@@ -185,9 +207,11 @@ limitations under the License.
size_delta_deleted: -5, size_delta_deleted: -5,
total_size: 220, total_size: 220,
}); });
assert.isFalse(element._hideBinaryChangeTotals);
assert.isFalse(element._hideChangeTotals);
}); });
test('_formatBytes function', function(){ test('_formatBytes function', function() {
var table = { var table = {
64: '+64 B', 64: '+64 B',
1023: '+1023 B', 1023: '+1023 B',
@@ -200,7 +224,7 @@ limitations under the License.
'-4096': '-4 KiB', '-4096': '-4 KiB',
'-1073741824': '-1 GiB', '-1073741824': '-1 GiB',
0: '+/-0 B', 0: '+/-0 B',
} };
for (var bytes in table) { for (var bytes in table) {
if (table.hasOwnProperty(bytes)) { if (table.hasOwnProperty(bytes)) {
@@ -209,7 +233,7 @@ limitations under the License.
} }
}); });
test('_formatPercentage function', function(){ test('_formatPercentage function', function() {
var table = [ var table = [
{ size: 100, { size: 100,
delta: 100, delta: 100,
@@ -235,7 +259,7 @@ limitations under the License.
delta: 10, delta: 10,
display: '(+10%)', display: '(+10%)',
}, },
] ];
table.forEach(function(item) { table.forEach(function(item) {
assert.equal(element._formatPercentage( assert.equal(element._formatPercentage(