From 8917b834eb11efb900558b27cab7900218822adc Mon Sep 17 00:00:00 2001 From: Hamza Nasser <69948099+Hamza-Nasser@users.noreply.github.com> Date: Sat, 5 Apr 2025 13:11:02 +0200 Subject: [PATCH 1/4] [google_identity_services_web] Enhance maybeEnum function to handle ArgumentError and add comprehensive tests --- .../lib/src/js_interop/shared.dart | 10 +++- .../test/src/js_interop/shared_test.dart | 55 +++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 packages/google_identity_services_web/test/src/js_interop/shared_test.dart diff --git a/packages/google_identity_services_web/lib/src/js_interop/shared.dart b/packages/google_identity_services_web/lib/src/js_interop/shared.dart index a56450383261..d542ba0dc198 100644 --- a/packages/google_identity_services_web/lib/src/js_interop/shared.dart +++ b/packages/google_identity_services_web/lib/src/js_interop/shared.dart @@ -3,11 +3,19 @@ // found in the LICENSE file. /// Attempts to retrieve an enum value from [haystack] if [needle] is not null. +/// Returns null if [needle] is null or if the enum value is not found in +/// [haystack]. T? maybeEnum(String? needle, List haystack) { if (needle == null) { return null; } - return haystack.byName(needle); + + try { + return haystack.byName(needle); + } on ArgumentError catch (_) { + // Ignore the error and return null. + return null; + } } /// The type of several functions from the library, that don't receive diff --git a/packages/google_identity_services_web/test/src/js_interop/shared_test.dart b/packages/google_identity_services_web/test/src/js_interop/shared_test.dart new file mode 100644 index 000000000000..73d08e8b2b44 --- /dev/null +++ b/packages/google_identity_services_web/test/src/js_interop/shared_test.dart @@ -0,0 +1,55 @@ +import 'package:google_identity_services_web/src/js_interop/shared.dart' + show maybeEnum; +import 'package:test/test.dart'; + +// Define a sample enum for testing +enum TestEnum { + first, + second, + third, + camelCase, + UPPERCASE, +} + +void main() { + group('maybeEnum tests', () { + test('should return null when needle is null', () { + expect(maybeEnum(null, TestEnum.values), isNull); + }); + + test('should return the correct enum when needle exists', () { + expect(maybeEnum('first', TestEnum.values), equals(TestEnum.first)); + expect(maybeEnum('second', TestEnum.values), equals(TestEnum.second)); + expect(maybeEnum('third', TestEnum.values), equals(TestEnum.third)); + }); + + test('should handle different casing in enum names', () { + expect(maybeEnum('camelCase', TestEnum.values), equals(TestEnum.camelCase)); + expect(maybeEnum('UPPERCASE', TestEnum.values), equals(TestEnum.UPPERCASE)); + }); + + test('should return null when needle is not found', () { + expect(maybeEnum('notFound', TestEnum.values), isNull); + expect(maybeEnum('Fourth', TestEnum.values), isNull); + expect(maybeEnum('', TestEnum.values), isNull); + }); + + test('should return null for case-mismatched enum names', () { + expect(maybeEnum('FIRST', TestEnum.values), isNull); + expect(maybeEnum('camelcase', TestEnum.values), isNull); + expect(maybeEnum('uppercase', TestEnum.values), isNull); + }); + + test('should handle special characters in needle', () { + expect(maybeEnum('first!', TestEnum.values), isNull); + expect(maybeEnum(' first ', TestEnum.values), isNull); + }); + + test('should work with empty enum values list', () { + // This is a theoretical test - in practice, enum values are never empty + // But the function should handle this case gracefully + final emptyList = []; + expect(maybeEnum('anything', emptyList), isNull); + }); + }); +} From ff6a4469e1d26daa2f31e103e017642b87b07486 Mon Sep 17 00:00:00 2001 From: Hamza Nasser <69948099+Hamza-Nasser@users.noreply.github.com> Date: Sat, 5 Apr 2025 13:13:14 +0200 Subject: [PATCH 2/4] Refactor tests in shared_test.dart for better readability and remove theoretical test case for empty enum values --- .../test/src/js_interop/shared_test.dart | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/packages/google_identity_services_web/test/src/js_interop/shared_test.dart b/packages/google_identity_services_web/test/src/js_interop/shared_test.dart index 73d08e8b2b44..071f0497263c 100644 --- a/packages/google_identity_services_web/test/src/js_interop/shared_test.dart +++ b/packages/google_identity_services_web/test/src/js_interop/shared_test.dart @@ -24,8 +24,10 @@ void main() { }); test('should handle different casing in enum names', () { - expect(maybeEnum('camelCase', TestEnum.values), equals(TestEnum.camelCase)); - expect(maybeEnum('UPPERCASE', TestEnum.values), equals(TestEnum.UPPERCASE)); + expect( + maybeEnum('camelCase', TestEnum.values), equals(TestEnum.camelCase)); + expect( + maybeEnum('UPPERCASE', TestEnum.values), equals(TestEnum.UPPERCASE)); }); test('should return null when needle is not found', () { @@ -39,17 +41,10 @@ void main() { expect(maybeEnum('camelcase', TestEnum.values), isNull); expect(maybeEnum('uppercase', TestEnum.values), isNull); }); - + test('should handle special characters in needle', () { expect(maybeEnum('first!', TestEnum.values), isNull); expect(maybeEnum(' first ', TestEnum.values), isNull); }); - - test('should work with empty enum values list', () { - // This is a theoretical test - in practice, enum values are never empty - // But the function should handle this case gracefully - final emptyList = []; - expect(maybeEnum('anything', emptyList), isNull); - }); }); } From 5fcadadcd4dc153ca03e9256ff5f76f4ce245a7e Mon Sep 17 00:00:00 2001 From: Hamza Nasser <69948099+Hamza-Nasser@users.noreply.github.com> Date: Tue, 22 Apr 2025 11:54:05 +0200 Subject: [PATCH 3/4] chore: bump version to 0.3.3+1 and update CHANGELOG for maybeEnum robustness improvement --- packages/google_identity_services_web/CHANGELOG.md | 4 ++++ packages/google_identity_services_web/pubspec.yaml | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/google_identity_services_web/CHANGELOG.md b/packages/google_identity_services_web/CHANGELOG.md index 74323f88c30e..a348d5ab6126 100644 --- a/packages/google_identity_services_web/CHANGELOG.md +++ b/packages/google_identity_services_web/CHANGELOG.md @@ -1,3 +1,7 @@ +## 0.3.3+1 + +* Improves robustness of `maybeEnum` by catching `ArgumentError` when the enum name is not found and returning `null`. + ## 0.3.3 * Moves all the JavaScript types to extend `JSObject`. diff --git a/packages/google_identity_services_web/pubspec.yaml b/packages/google_identity_services_web/pubspec.yaml index cedb0e5b194b..4433fc6c6e07 100644 --- a/packages/google_identity_services_web/pubspec.yaml +++ b/packages/google_identity_services_web/pubspec.yaml @@ -2,7 +2,7 @@ name: google_identity_services_web description: A Dart JS-interop layer for Google Identity Services. Google's new sign-in SDK for Web that supports multiple types of credentials. repository: https://github.com/flutter/packages/tree/main/packages/google_identity_services_web issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+google_identiy_services_web%22 -version: 0.3.3 +version: 0.3.3+1 environment: sdk: ^3.4.0 From 801c3737084e8797c30e25d17d12d49774c3ca56 Mon Sep 17 00:00:00 2001 From: Hamza Nasser <69948099+Hamza-Nasser@users.noreply.github.com> Date: Tue, 22 Apr 2025 11:56:00 +0200 Subject: [PATCH 4/4] refactor: clean up unnecessary comments --- .../lib/src/js_interop/shared.dart | 3 +-- .../test/src/js_interop/shared_test.dart | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/google_identity_services_web/lib/src/js_interop/shared.dart b/packages/google_identity_services_web/lib/src/js_interop/shared.dart index d542ba0dc198..448ca006a429 100644 --- a/packages/google_identity_services_web/lib/src/js_interop/shared.dart +++ b/packages/google_identity_services_web/lib/src/js_interop/shared.dart @@ -9,11 +9,10 @@ T? maybeEnum(String? needle, List haystack) { if (needle == null) { return null; } - + try { return haystack.byName(needle); } on ArgumentError catch (_) { - // Ignore the error and return null. return null; } } diff --git a/packages/google_identity_services_web/test/src/js_interop/shared_test.dart b/packages/google_identity_services_web/test/src/js_interop/shared_test.dart index 071f0497263c..fdaa0dbaca19 100644 --- a/packages/google_identity_services_web/test/src/js_interop/shared_test.dart +++ b/packages/google_identity_services_web/test/src/js_interop/shared_test.dart @@ -2,7 +2,6 @@ import 'package:google_identity_services_web/src/js_interop/shared.dart' show maybeEnum; import 'package:test/test.dart'; -// Define a sample enum for testing enum TestEnum { first, second,