From aa5a50fd5fcdb4bd9b18ce9731f82a228b810420 Mon Sep 17 00:00:00 2001 From: Wink Saville Date: Tue, 5 May 2020 23:24:09 -0700 Subject: [PATCH] Fix issues in getMinPixels and getMaxPixels The getMinPixels and getMaxPixels methods will never assert, return null or return a negative value. Remove assert statements in getMinPixels and getMaxPixels. The assert in gitMinPixel was asserting if an application on an Android device was run while the display was off because totalPixels was 0. Handle totalPixels parameter being null. --- .../lib/src/chart/layout/layout_config.dart | 34 +- .../layout/layout_manager_impl_test.dart | 366 ++++++++++++++++++ 2 files changed, 392 insertions(+), 8 deletions(-) diff --git a/charts_common/lib/src/chart/layout/layout_config.dart b/charts_common/lib/src/chart/layout/layout_config.dart index 1ef635e39..2859bbf13 100644 --- a/charts_common/lib/src/chart/layout/layout_config.dart +++ b/charts_common/lib/src/chart/layout/layout_config.dart @@ -13,6 +13,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +import 'dart:math' show min; + /// Collection of configurations that apply to the [LayoutManager]. class LayoutConfig { final MarginSpec leftSpec; @@ -97,25 +99,41 @@ class MarginSpec { /// Get the min pixels, given the [totalPixels]. int getMinPixels(int totalPixels) { + int result; + + if (totalPixels == null) { + totalPixels = 0; + } + + // All paths must set result if (_minPixel != null) { - assert(_minPixel < totalPixels); - return _minPixel; + result = min(_minPixel, totalPixels); } else if (_minPercent != null) { - return (totalPixels * (_minPercent / 100)).round(); + result = (totalPixels * (_minPercent / 100)).round(); } else { - return 0; + result = 0; } + + return result >= 0 ? result : 0; } /// Get the max pixels, given the [totalPixels]. int getMaxPixels(int totalPixels) { + int result; + + if (totalPixels == null) { + totalPixels = 0; + } + + // All paths must set result if (_maxPixel != null) { - assert(_maxPixel < totalPixels); - return _maxPixel; + result = min(_maxPixel, totalPixels); } else if (_maxPercent != null) { - return (totalPixels * (_maxPercent / 100)).round(); + result = (totalPixels * (_maxPercent / 100)).round(); } else { - return totalPixels; + result = totalPixels; } + + return result >= 0 ? result : 0; } } diff --git a/charts_common/test/chart/layout/layout_manager_impl_test.dart b/charts_common/test/chart/layout/layout_manager_impl_test.dart index 88f0a8a99..b9782fab2 100644 --- a/charts_common/test/chart/layout/layout_manager_impl_test.dart +++ b/charts_common/test/chart/layout/layout_manager_impl_test.dart @@ -45,4 +45,370 @@ void main() { expect(layout.marginBottom, equals(10)); expect(layout.marginLeft, equals(9)); }); + + test('marginSpec.fromPixel default', () { + var ms = MarginSpec.fromPixel(); + + expect(ms.getMinPixels(null), equals(0)); + expect(ms.getMinPixels(-1), equals(0)); + expect(ms.getMinPixels(0), equals(0)); + expect(ms.getMinPixels(1), equals(0)); + expect(ms.getMinPixels(1000000), equals(0)); + + expect(ms.getMaxPixels(null), equals(0)); + expect(ms.getMaxPixels(-1), equals(0)); + expect(ms.getMaxPixels(0), equals(0)); + expect(ms.getMaxPixels(1), equals(1)); + expect(ms.getMaxPixels(1000000), equals(1000000)); + }); + + test('marginSpec.fromPixel(minPixel: -1)', () { + // This didn't work :( + //expect(MarginSpec.fromPixel(minPixel: -1), throwsA(isA())); + + try { + MarginSpec.fromPixel(minPixel: -1); + expect(false, equals(true), reason: 'Expected assert error, run with: `pub run test xxx` or `dart --enable-asserts xxx`'); + } catch (e) { + //print('minPixel: caught error'); + expect(e.runtimeType.toString(), equals('_AssertionError')); + } + }); + + test('marginSpec.fromPixel(maxPixel: -1)', () { + // This didn't work :( + //expect(MarginSpec.fromPixel(maxPixel: -1), throwsA(isA())); + + try { + MarginSpec.fromPixel(maxPixel: -1); + expect(false, equals(true), reason: 'Expected assert error, run with: `pub run test xxx` or `dart --enable-asserts xxx`'); + } catch (e) { + //print('minPixel: caught error'); + expect(e.runtimeType.toString(), equals('_AssertionError')); + } + }); + + test('marginSpec.fromPixel(minPixel: 0, maxPixel: -1)', () { + // This didn't work :( + //expect(MarginSpec.fromPixel(minPixel: 0, maxPixel: -1), throwsA(isA())); + + try { + MarginSpec.fromPixel(minPixel: 0, maxPixel: -1); + expect(false, equals(true), reason: 'Expected assert error, run with: `pub run test xxx` or `dart --enable-asserts xxx`'); + } catch (e) { + //print('minPixel: caught error'); + expect(e.runtimeType.toString(), equals('_AssertionError')); + } + }); + + test('marginSpec.fromPixel(minPixel: -1, maxPixel: 0)', () { + // This didn't work :( + //expect(MarginSpec.fromPixel(minPixel: -1, maxPixel: 0), throwsA(isA())); + + try { + MarginSpec.fromPixel(minPixel: -1, maxPixel: 0); + expect(false, equals(true), reason: 'Expected assert error, run with: `pub run test xxx` or `dart --enable-asserts xxx`'); + } catch (e) { + //print('minPixel: caught error'); + expect(e.runtimeType.toString(), equals('_AssertionError')); + } + }); + + test('marginSpec.fromPixel(minPixel: -1, maxPixel: -1)', () { + // This didn't work :( + //expect(MarginSpec.fromPixel(minPixel: -1, maxPixel: -1), throwsA(isA())); + + try { + MarginSpec.fromPixel(minPixel: -1, maxPixel: -1); + expect(false, equals(true), reason: 'Expected assert error, run with: `pub run test xxx` or `dart --enable-asserts xxx`'); + } catch (e) { + //print('minPixel: caught error'); + expect(e.runtimeType.toString(), equals('_AssertionError')); + } + }); + + test('marginSpec.fromPixel(minPixel: 0)', () { + var ms = MarginSpec.fromPixel(minPixel: 0); + + expect(ms.getMinPixels(null), equals(0)); + expect(ms.getMinPixels(-1), equals(0)); + expect(ms.getMinPixels(0), equals(0)); + expect(ms.getMinPixels(1), equals(0)); + expect(ms.getMinPixels(1000000), equals(0)); + + expect(ms.getMaxPixels(null), equals(0)); + expect(ms.getMaxPixels(-1), equals(0)); + expect(ms.getMaxPixels(0), equals(0)); + expect(ms.getMaxPixels(1), equals(1)); + expect(ms.getMaxPixels(1000000), equals(1000000)); + }); + + test('marginSpec.fromPixel(minPixel: 1)', () { + var ms = MarginSpec.fromPixel(minPixel: 1); + + expect(ms.getMinPixels(null), equals(0)); + expect(ms.getMinPixels(-1), equals(0)); + expect(ms.getMinPixels(0), equals(0)); + expect(ms.getMinPixels(1), equals(1)); + expect(ms.getMinPixels(1000000), equals(1)); + + expect(ms.getMaxPixels(null), equals(0)); + expect(ms.getMaxPixels(-1), equals(0)); + expect(ms.getMaxPixels(0), equals(0)); + expect(ms.getMaxPixels(1), equals(1)); + expect(ms.getMaxPixels(1000000), equals(1000000)); + }); + + test('marginSpec.fromPixel(minPixel: 100)', () { + var ms = MarginSpec.fromPixel(minPixel: 100); + + expect(ms.getMinPixels(null), equals(0)); + expect(ms.getMinPixels(-1), equals(0)); + expect(ms.getMinPixels(0), equals(0)); + expect(ms.getMinPixels(1), equals(1)); + expect(ms.getMinPixels(1000000), equals(100)); + + expect(ms.getMaxPixels(null), equals(0)); + expect(ms.getMaxPixels(-1), equals(0)); + expect(ms.getMaxPixels(0), equals(0)); + expect(ms.getMaxPixels(1), equals(1)); + expect(ms.getMaxPixels(1000000), equals(1000000)); + }); + + test('marginSpec.fromPixel(maxPixel: 100)', () { + var ms = MarginSpec.fromPixel(maxPixel: 100); + + expect(ms.getMinPixels(null), equals(0)); + expect(ms.getMinPixels(-1), equals(0)); + expect(ms.getMinPixels(0), equals(0)); + expect(ms.getMinPixels(1), equals(0)); + expect(ms.getMinPixels(1000000), equals(0)); + + expect(ms.getMaxPixels(null), equals(0)); + expect(ms.getMaxPixels(-1), equals(0)); + expect(ms.getMaxPixels(0), equals(0)); + expect(ms.getMaxPixels(1), equals(1)); + expect(ms.getMaxPixels(1000000), equals(100)); + }); + + test('marginSpec.fromPixel(minPixel: 50, maxPixel: 100)', () { + var ms = MarginSpec.fromPixel(minPixel: 50, maxPixel: 100); + + expect(ms.getMinPixels(null), equals(0)); + expect(ms.getMinPixels(-1), equals(0)); + expect(ms.getMinPixels(0), equals(0)); + expect(ms.getMinPixels(1), equals(1)); + expect(ms.getMinPixels(1000000), equals(50)); + + expect(ms.getMaxPixels(null), equals(0)); + expect(ms.getMaxPixels(-1), equals(0)); + expect(ms.getMaxPixels(0), equals(0)); + expect(ms.getMaxPixels(1), equals(1)); + expect(ms.getMaxPixels(1000000), equals(100)); + }); + + test('marginSpec.fromPercent default', () { + var ms = MarginSpec.fromPercent(); + + expect(ms.getMinPixels(null), equals(0)); + expect(ms.getMinPixels(-1), equals(0)); + expect(ms.getMinPixels(0), equals(0)); + expect(ms.getMinPixels(1), equals(0)); + expect(ms.getMinPixels(1000000), equals(0)); + + expect(ms.getMaxPixels(null), equals(0)); + expect(ms.getMaxPixels(-1), equals(0)); + expect(ms.getMaxPixels(0), equals(0)); + expect(ms.getMaxPixels(1), equals(1)); + expect(ms.getMaxPixels(1000000), equals(1000000)); + }); + + test('marginSpec.fromPercent(minPercent: -1)', () { + // This didn't work :( + //expect(MarginSpec.fromPercent(minPercent: -1), throwsA(isA())); + + try { + MarginSpec.fromPercent(minPercent: -1); + expect(false, equals(true), reason: 'Expected assert error, run with: `pub run test xxx` or `dart --enable-asserts xxx`'); + } catch (e) { + //print('minPercent: caught error'); + expect(e.runtimeType.toString(), equals('_AssertionError')); + } + }); + + test('marginSpec.fromPercent(maxPercent: -1)', () { + // This didn't work :( + //expect(MarginSpec.fromPercent(maxPercent: -1), throwsA(isA())); + + try { + MarginSpec.fromPercent(maxPercent: -1); + expect(false, equals(true), reason: 'Expected assert error, run with: `pub run test xxx` or `dart --enable-asserts xxx`'); + } catch (e) { + //print('minPercent: caught error'); + expect(e.runtimeType.toString(), equals('_AssertionError')); + } + }); + + test('marginSpec.fromPercent(minPercent: 0, maxPercent: -1)', () { + // This didn't work :( + //expect(MarginSpec.fromPercent(minPercent: 0, maxPercent: -1), throwsA(isA())); + + try { + MarginSpec.fromPercent(minPercent: 0, maxPercent: -1); + expect(false, equals(true), reason: 'Expected assert error, run with: `pub run test xxx` or `dart --enable-asserts xxx`'); + } catch (e) { + //print('minPercent: caught error'); + expect(e.runtimeType.toString(), equals('_AssertionError')); + } + }); + + test('marginSpec.fromPercent(minPercent: -1, maxPercent: 0)', () { + // This didn't work :( + //expect(MarginSpec.fromPercent(minPercent: -1, maxPercent: 0), throwsA(isA())); + + try { + MarginSpec.fromPercent(minPercent: -1, maxPercent: 0); + expect(false, equals(true), reason: 'Expected assert error, run with: `pub run test xxx` or `dart --enable-asserts xxx`'); + } catch (e) { + //print('minPercent: caught error'); + expect(e.runtimeType.toString(), equals('_AssertionError')); + } + }); + + test('marginSpec.fromPercent(minPercent: -1, maxPercent: -1)', () { + // This didn't work :( + //expect(MarginSpec.fromPercent(minPercent: -1, maxPercent: -1), throwsA(isA())); + + try { + MarginSpec.fromPercent(minPercent: -1, maxPercent: -1); + expect(false, equals(true), reason: 'Expected assert error, run with: `pub run test xxx` or `dart --enable-asserts xxx`'); + } catch (e) { + //print('minPercent: caught error'); + expect(e.runtimeType.toString(), equals('_AssertionError')); + } + }); + + test('marginSpec.fromPercent(minPercent: 0)', () { + var ms = MarginSpec.fromPercent(minPercent: 0); + + expect(ms.getMinPixels(null), equals(0)); + expect(ms.getMinPixels(-1), equals(0)); + expect(ms.getMinPixels(0), equals(0)); + expect(ms.getMinPixels(1), equals(0)); + expect(ms.getMinPixels(1000000), equals(0)); + + expect(ms.getMaxPixels(null), equals(0)); + expect(ms.getMaxPixels(-1), equals(0)); + expect(ms.getMaxPixels(0), equals(0)); + expect(ms.getMaxPixels(1), equals(1)); + expect(ms.getMaxPixels(1000000), equals(1000000)); + }); + + test('marginSpec.fromPercent(minPercent: 1)', () { + var ms = MarginSpec.fromPercent(minPercent: 1); + + expect(ms.getMinPixels(null), equals(0)); + expect(ms.getMinPixels(-1), equals(0)); + expect(ms.getMinPixels(0), equals(0)); + expect(ms.getMinPixels(1), equals(0)); + expect(ms.getMinPixels(1000000), equals(10000)); + + expect(ms.getMaxPixels(null), equals(0)); + expect(ms.getMaxPixels(-1), equals(0)); + expect(ms.getMaxPixels(0), equals(0)); + expect(ms.getMaxPixels(1), equals(1)); + expect(ms.getMaxPixels(1000000), equals(1000000)); + }); + + test('marginSpec.fromPercent(minPercent: 100)', () { + var ms = MarginSpec.fromPercent(minPercent: 100); + + expect(ms.getMinPixels(null), equals(0)); + expect(ms.getMinPixels(-1), equals(0)); + expect(ms.getMinPixels(0), equals(0)); + expect(ms.getMinPixels(1), equals(1)); + expect(ms.getMinPixels(1000000), equals(1000000)); + + expect(ms.getMaxPixels(null), equals(0)); + expect(ms.getMaxPixels(-1), equals(0)); + expect(ms.getMaxPixels(0), equals(0)); + expect(ms.getMaxPixels(1), equals(1)); + expect(ms.getMaxPixels(1000000), equals(1000000)); + }); + + test('marginSpec.fromPercent(minPercent: 50, maxPercent: 100)', () { + var ms = MarginSpec.fromPercent(minPercent: 50, maxPercent: 100); + + expect(ms.getMinPixels(null), equals(0)); + expect(ms.getMinPixels(-1), equals(0)); + expect(ms.getMinPixels(0), equals(0)); + expect(ms.getMinPixels(1), equals(1)); + expect(ms.getMinPixels(1000000), equals(500000)); + + expect(ms.getMaxPixels(null), equals(0)); + expect(ms.getMaxPixels(-1), equals(0)); + expect(ms.getMaxPixels(0), equals(0)); + expect(ms.getMaxPixels(1), equals(1)); + expect(ms.getMaxPixels(1000000), equals(1000000)); + }); + + test('marginSpec.fixedPixel(null) ', () { + var ms = MarginSpec.fixedPixel(null); + + expect(ms.getMinPixels(null), equals(0)); + expect(ms.getMinPixels(-1), equals(0)); + expect(ms.getMinPixels(0), equals(0)); + expect(ms.getMinPixels(1), equals(0)); + expect(ms.getMinPixels(1000000), equals(0)); + + expect(ms.getMaxPixels(null), equals(0)); + expect(ms.getMaxPixels(-1), equals(0)); + expect(ms.getMaxPixels(0), equals(0)); + expect(ms.getMaxPixels(1), equals(1)); + expect(ms.getMaxPixels(1000000), equals(1000000)); + }); + + test('marginSpec.fixedPixel(-1) ', () { + // This didn't work :( + //expect(MarginSpec.fixedPixel(-1), throwsA(isA())); + + try { + MarginSpec.fixedPixel(-1); + expect(false, equals(true), reason: 'Expected assert error, run with: `pub run test xxx` or `dart --enable-asserts xxx`'); + } catch (e) { + expect(e.runtimeType.toString(), equals('_AssertionError')); + } + }); + + test('marginSpec.fixedPixel(0) ', () { + var ms = MarginSpec.fixedPixel(0); + + expect(ms.getMinPixels(null), equals(0)); + expect(ms.getMinPixels(-1), equals(0)); + expect(ms.getMinPixels(0), equals(0)); + expect(ms.getMinPixels(1), equals(0)); + expect(ms.getMinPixels(1000000), equals(0)); + + expect(ms.getMaxPixels(null), equals(0)); + expect(ms.getMaxPixels(-1), equals(0)); + expect(ms.getMaxPixels(0), equals(0)); + expect(ms.getMaxPixels(1), equals(0)); + expect(ms.getMaxPixels(1000000), equals(0)); + }); + + test('marginSpec.fixedPixel(100) ', () { + var ms = MarginSpec.fixedPixel(100); + + expect(ms.getMinPixels(null), equals(0)); + expect(ms.getMinPixels(-1), equals(0)); + expect(ms.getMinPixels(0), equals(0)); + expect(ms.getMinPixels(1), equals(1)); + expect(ms.getMinPixels(1000000), equals(100)); + + expect(ms.getMaxPixels(null), equals(0)); + expect(ms.getMaxPixels(-1), equals(0)); + expect(ms.getMaxPixels(0), equals(0)); + expect(ms.getMaxPixels(1), equals(1)); + expect(ms.getMaxPixels(1000000), equals(100)); + }); }