From 71fac07a930ab0137a110bc34eccf5ef6d26fa43 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Thu, 8 Sep 2016 13:31:01 -0700 Subject: [PATCH 1/6] Add scale factor assert helpers --- spec/api-browser-window-spec.js | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index ba8c979a9f8b..408556031466 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -1359,3 +1359,33 @@ describe('browser-window module', function () { }) }) }) + +const assertPositionsEqual = (expect, actual) => { + if (isIntegerScaleFactor()) { + assert.deepEqual(expect, actual) + } else { + assertWithinDelta(expect[0], actual[0], 1, 'x') + assertWithinDelta(expect[1], actual[1], 1, 'y') + } +} + +const assertBoundsEqual = (expect, actual) => { + if (isIntegerScaleFactor()) { + assert.deepEqual(expect, actual) + } else { + assertWithinDelta(expect.x, actual.x, 1, 'x') + assertWithinDelta(expect.y, actual.y, 1, 'y') + assertWithinDelta(expect.width, actual.width, 1, 'width') + assertWithinDelta(expect.height, actual.height, 1, 'height') + } +} + +const assertWithinDelta = (expect, actual, delta, label) => { + const result = Math.abs(actual[0] - expect[0]) + assert.ok(result <= delta, `${label} value of ${expect} was not within ${delta} of ${actual}`) +} + +const isIntegerScaleFactor = () => { + const {scaleFactor} = screen.getPrimaryDisplay() + return Math.round(scaleFactor) === scaleFactor +} From d88f70caa2fba505caac29db87de7f4cca176b06 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Thu, 8 Sep 2016 13:33:43 -0700 Subject: [PATCH 2/6] Use single assert bounds helper --- spec/api-browser-window-spec.js | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index 408556031466..3cb9db04f0db 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -1360,18 +1360,12 @@ describe('browser-window module', function () { }) }) -const assertPositionsEqual = (expect, actual) => { - if (isIntegerScaleFactor()) { - assert.deepEqual(expect, actual) - } else { - assertWithinDelta(expect[0], actual[0], 1, 'x') - assertWithinDelta(expect[1], actual[1], 1, 'y') - } -} - const assertBoundsEqual = (expect, actual) => { if (isIntegerScaleFactor()) { assert.deepEqual(expect, actual) + } else if (Array.isArray(actual)) { + assertWithinDelta(expect[0], actual[0], 1, 'x') + assertWithinDelta(expect[1], actual[1], 1, 'y') } else { assertWithinDelta(expect.x, actual.x, 1, 'x') assertWithinDelta(expect.y, actual.y, 1, 'y') From 7494e286d979ccfd43faaa5c9a6f6334136460c2 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Thu, 8 Sep 2016 13:39:15 -0700 Subject: [PATCH 3/6] Put actual value first --- spec/api-browser-window-spec.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index 3cb9db04f0db..125aba9b752b 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -1360,21 +1360,21 @@ describe('browser-window module', function () { }) }) -const assertBoundsEqual = (expect, actual) => { +const assertBoundsEqual = (actual, expect) => { if (isIntegerScaleFactor()) { assert.deepEqual(expect, actual) } else if (Array.isArray(actual)) { - assertWithinDelta(expect[0], actual[0], 1, 'x') - assertWithinDelta(expect[1], actual[1], 1, 'y') + assertWithinDelta(actual[0], expect[0], 1, 'x') + assertWithinDelta(actual[1], expect[1], 1, 'y') } else { - assertWithinDelta(expect.x, actual.x, 1, 'x') - assertWithinDelta(expect.y, actual.y, 1, 'y') - assertWithinDelta(expect.width, actual.width, 1, 'width') - assertWithinDelta(expect.height, actual.height, 1, 'height') + assertWithinDelta(actual.x, expect.x, 1, 'x') + assertWithinDelta(actual.y, expect.y, 1, 'y') + assertWithinDelta(actual.width, expect.width, 1, 'width') + assertWithinDelta(actual.height, expect.height, 1, 'height') } } -const assertWithinDelta = (expect, actual, delta, label) => { +const assertWithinDelta = (actual, expect, delta, label) => { const result = Math.abs(actual[0] - expect[0]) assert.ok(result <= delta, `${label} value of ${expect} was not within ${delta} of ${actual}`) } From 59a49f6f963c030b7fe5476f22d6c96ca35312c0 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Thu, 8 Sep 2016 13:46:55 -0700 Subject: [PATCH 4/6] Use bounds helper in failing specs --- spec/api-browser-window-spec.js | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index 125aba9b752b..d8304e51a7d4 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -273,9 +273,7 @@ describe('browser-window module', function () { it('sets the window size', function (done) { var size = [300, 400] w.once('resize', function () { - var newSize = w.getSize() - assert.equal(newSize[0], size[0]) - assert.equal(newSize[1], size[1]) + assertBoundsEqual(w.getSize(), size) done() }) w.setSize(size[0], size[1]) @@ -288,12 +286,12 @@ describe('browser-window module', function () { assert.deepEqual(w.getMaximumSize(), [0, 0]) w.setMinimumSize(100, 100) - assert.deepEqual(w.getMinimumSize(), [100, 100]) - assert.deepEqual(w.getMaximumSize(), [0, 0]) + assertBoundsEqual(w.getMinimumSize(), [100, 100]) + assertBoundsEqual(w.getMaximumSize(), [0, 0]) w.setMaximumSize(900, 600) - assert.deepEqual(w.getMinimumSize(), [100, 100]) - assert.deepEqual(w.getMaximumSize(), [900, 600]) + assertBoundsEqual(w.getMinimumSize(), [100, 100]) + assertBoundsEqual(w.getMaximumSize(), [900, 600]) }) }) @@ -303,9 +301,7 @@ describe('browser-window module', function () { w.setAspectRatio(1 / 2) w.setAspectRatio(0) w.once('resize', function () { - var newSize = w.getSize() - assert.equal(newSize[0], size[0]) - assert.equal(newSize[1], size[1]) + assertBoundsEqual(w.getSize(), size) done() }) w.setSize(size[0], size[1]) @@ -354,7 +350,7 @@ describe('browser-window module', function () { it('sets the content size and position', function (done) { var bounds = {x: 10, y: 10, width: 250, height: 250} w.once('resize', function () { - assert.deepEqual(w.getContentBounds(), bounds) + assertBoundsEqual(w.getContentBounds(), bounds) done() }) w.setContentBounds(bounds) @@ -515,9 +511,7 @@ describe('browser-window module', function () { size.width += 100 size.height += 100 w.setSize(size.width, size.height) - var after = w.getSize() - assert.equal(after[0], size.width) - assert.equal(after[1], size.height) + assertBoundsEqual(w.getSize(), [size.width, size.height]) }) }) @@ -1375,7 +1369,7 @@ const assertBoundsEqual = (actual, expect) => { } const assertWithinDelta = (actual, expect, delta, label) => { - const result = Math.abs(actual[0] - expect[0]) + const result = Math.abs(actual - expect) assert.ok(result <= delta, `${label} value of ${expect} was not within ${delta} of ${actual}`) } From 68c67b64c5a3a31fce0d0fe897fdbf0b36b71c2a Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Thu, 8 Sep 2016 16:24:11 -0700 Subject: [PATCH 5/6] Use delta when scale factor is above 2 and odd --- spec/api-browser-window-spec.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index d8304e51a7d4..e3e482002875 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -1355,7 +1355,7 @@ describe('browser-window module', function () { }) const assertBoundsEqual = (actual, expect) => { - if (isIntegerScaleFactor()) { + if (!isScaleFactorRounding()) { assert.deepEqual(expect, actual) } else if (Array.isArray(actual)) { assertWithinDelta(actual[0], expect[0], 1, 'x') @@ -1373,7 +1373,12 @@ const assertWithinDelta = (actual, expect, delta, label) => { assert.ok(result <= delta, `${label} value of ${expect} was not within ${delta} of ${actual}`) } -const isIntegerScaleFactor = () => { +// Is the display's scale factor possibly causing rounding of pixel coordinate +// values? +const isScaleFactorRounding = () => { const {scaleFactor} = screen.getPrimaryDisplay() - return Math.round(scaleFactor) === scaleFactor + // Return true if scale factor is non-integer value + if (Math.round(scaleFactor) === scaleFactor) return true + // Return true if scale factor is odd number above 2 + return scaleFactor > 2 && scaleFactor % 2 === 1 } From 7c26fe46b8c9d5e38ed6455d7c756b20c8324880 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Thu, 8 Sep 2016 17:12:53 -0700 Subject: [PATCH 6/6] === -> !== --- spec/api-browser-window-spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index e3e482002875..1689ca759598 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -1378,7 +1378,7 @@ const assertWithinDelta = (actual, expect, delta, label) => { const isScaleFactorRounding = () => { const {scaleFactor} = screen.getPrimaryDisplay() // Return true if scale factor is non-integer value - if (Math.round(scaleFactor) === scaleFactor) return true + if (Math.round(scaleFactor) !== scaleFactor) return true // Return true if scale factor is odd number above 2 return scaleFactor > 2 && scaleFactor % 2 === 1 }