From e791391cde2d507e728013c57026d8b959d7eae3 Mon Sep 17 00:00:00 2001 From: Felix Angelov Date: Fri, 14 Apr 2023 11:22:30 -0500 Subject: [PATCH] refactor(shorebird_cli): auth tests should use temporary directory --- packages/shorebird_cli/lib/src/auth/auth.dart | 14 ++- .../shorebird_cli/lib/src/config/config.dart | 1 - .../lib/src/config/shorebird_config_dir.dart | 13 --- .../test/src/auth/auth_test.dart | 89 ++++++++++--------- .../test/src/commands/build_command_test.dart | 4 - .../test/src/commands/login_command_test.dart | 3 - .../test/src/commands/patch_command_test.dart | 4 - .../src/commands/release_command_test.dart | 4 - .../test/src/commands/run_command_test.dart | 10 +-- 9 files changed, 61 insertions(+), 81 deletions(-) delete mode 100644 packages/shorebird_cli/lib/src/config/shorebird_config_dir.dart diff --git a/packages/shorebird_cli/lib/src/auth/auth.dart b/packages/shorebird_cli/lib/src/auth/auth.dart index 560489e92..7f06ce478 100644 --- a/packages/shorebird_cli/lib/src/auth/auth.dart +++ b/packages/shorebird_cli/lib/src/auth/auth.dart @@ -1,11 +1,12 @@ import 'dart:convert'; import 'dart:io'; +import 'package:cli_util/cli_util.dart'; import 'package:googleapis_auth/auth_io.dart' as oauth2; import 'package:http/http.dart' as http; import 'package:path/path.dart' as p; import 'package:shorebird_cli/src/auth/jwt.dart'; -import 'package:shorebird_cli/src/config/config.dart'; +import 'package:shorebird_cli/src/command_runner.dart'; final _clientId = oauth2.ClientId( /// Shorebird CLI's OAuth 2.0 identifier. @@ -77,18 +78,23 @@ class AuthenticatedClient extends http.BaseClient { class Auth { Auth({ http.Client? httpClient, + String? credentialsDir, ObtainAccessCredentials? obtainAccessCredentials, }) : _httpClient = httpClient ?? http.Client(), + _credentialsDir = + credentialsDir ?? applicationConfigHome(executableName), _obtainAccessCredentials = obtainAccessCredentials ?? oauth2.obtainAccessCredentialsViaUserConsent { _loadCredentials(); } - static const _credentialsFileName = 'credentials.json'; - final http.Client _httpClient; + final String _credentialsDir; final ObtainAccessCredentials _obtainAccessCredentials; - final credentialsFilePath = p.join(shorebirdConfigDir, _credentialsFileName); + + String get credentialsFilePath { + return p.join(_credentialsDir, 'credentials.json'); + } http.Client get client { final credentials = _credentials; diff --git a/packages/shorebird_cli/lib/src/config/config.dart b/packages/shorebird_cli/lib/src/config/config.dart index 8f842113f..fb8391af7 100644 --- a/packages/shorebird_cli/lib/src/config/config.dart +++ b/packages/shorebird_cli/lib/src/config/config.dart @@ -1,2 +1 @@ -export 'shorebird_config_dir.dart'; export 'shorebird_yaml.dart'; diff --git a/packages/shorebird_cli/lib/src/config/shorebird_config_dir.dart b/packages/shorebird_cli/lib/src/config/shorebird_config_dir.dart deleted file mode 100644 index 21b7115df..000000000 --- a/packages/shorebird_cli/lib/src/config/shorebird_config_dir.dart +++ /dev/null @@ -1,13 +0,0 @@ -import 'package:cli_util/cli_util.dart'; -import 'package:meta/meta.dart'; -import 'package:shorebird_cli/src/command_runner.dart'; - -final String shorebirdConfigDir = () { - final configHome = testApplicationConfigHome ?? applicationConfigHome; - return configHome(executableName); -}(); - -/// Test applicationConfigHome which should -/// only be used for testing purposes. -@visibleForTesting -String Function(String)? testApplicationConfigHome; diff --git a/packages/shorebird_cli/test/src/auth/auth_test.dart b/packages/shorebird_cli/test/src/auth/auth_test.dart index 085bba02f..988a238b2 100644 --- a/packages/shorebird_cli/test/src/auth/auth_test.dart +++ b/packages/shorebird_cli/test/src/auth/auth_test.dart @@ -10,41 +10,48 @@ class _FakeBaseRequest extends Fake implements http.BaseRequest {} class _MockHttpClient extends Mock implements http.Client {} -class _MockAccessCredentials extends Mock implements AccessCredentials {} - void main() { group('Auth', () { const idToken = '''eyJhbGciOiJSUzI1NiIsImN0eSI6IkpXVCJ9.eyJlbWFpbCI6InRlc3RAZW1haWwuY29tIn0.pD47BhF3MBLyIpfsgWCzP9twzC1HJxGukpcR36DqT6yfiOMHTLcjDbCjRLAnklWEHiT0BQTKTfhs8IousU90Fm5bVKObudfKu8pP5iZZ6Ls4ohDjTrXky9j3eZpZjwv8CnttBVgRfMJG-7YASTFRYFcOLUpnb4Zm5R6QdoCDUYg'''; const email = 'test@email.com'; - final validCredentials = AccessCredentials( - AccessToken( - 'Bearer', - 'accessToken', - DateTime.now().add(const Duration(minutes: 10)).toUtc(), - ), - '', - [], + const refreshToken = ''; + const scopes = []; + final accessToken = AccessToken( + 'Bearer', + 'accessToken', + DateTime.now().add(const Duration(minutes: 10)).toUtc(), + ); + + final accessCredentials = AccessCredentials( + accessToken, + refreshToken, + scopes, idToken: idToken, ); + late String credentialsDir; late http.Client httpClient; - late AccessCredentials accessCredentials; late Auth auth; setUpAll(() { registerFallbackValue(_FakeBaseRequest()); }); - setUp(() { - httpClient = _MockHttpClient(); - accessCredentials = _MockAccessCredentials(); - auth = Auth( + Auth buildAuth({AccessCredentials? credentials}) { + return Auth( + credentialsDir: credentialsDir, httpClient: httpClient, obtainAccessCredentials: (clientId, scopes, client, userPrompt) async { - return validCredentials; + return credentials ?? accessCredentials; }, - )..logout(); + ); + } + + setUp(() { + credentialsDir = Directory.systemTemp.createTempSync().path; + httpClient = _MockHttpClient(); + auth = buildAuth(); }); group('AuthenticatedClient', () { @@ -74,7 +81,7 @@ void main() { httpClient: httpClient, onRefreshCredentials: onRefreshCredentialsCalls.add, refreshCredentials: (clientId, credentials, client) async => - validCredentials, + accessCredentials, ); await client.get(Uri.parse('https://example.com')); @@ -100,7 +107,7 @@ void main() { ); final onRefreshCredentialsCalls = []; final client = AuthenticatedClient( - credentials: validCredentials, + credentials: accessCredentials, httpClient: httpClient, onRefreshCredentials: onRefreshCredentialsCalls.add, ); @@ -149,21 +156,20 @@ void main() { group('login', () { test('should set the user when claims are valid', () async { - when(() => accessCredentials.idToken).thenReturn(idToken); await auth.login((_) {}); expect(auth.email, email); expect(auth.isAuthenticated, isTrue); - expect(Auth().email, email); - expect(Auth().isAuthenticated, isTrue); + expect(buildAuth().email, email); + expect(buildAuth().isAuthenticated, isTrue); }); test('should not set the user when token is null', () async { - when(() => accessCredentials.idToken).thenReturn(null); - auth = Auth( - httpClient: httpClient, - obtainAccessCredentials: - (clientId, scopes, client, userPrompt) async => accessCredentials, + final credentialsWithNoIdToken = AccessCredentials( + accessToken, + refreshToken, + scopes, ); + auth = buildAuth(credentials: credentialsWithNoIdToken); await expectLater( auth.login((_) {}), throwsA( @@ -179,12 +185,13 @@ void main() { }); test('should not set the user when token is empty', () async { - when(() => accessCredentials.idToken).thenReturn(''); - auth = Auth( - httpClient: httpClient, - obtainAccessCredentials: - (clientId, scopes, client, userPrompt) async => accessCredentials, + final credentialsWithEmptyIdToken = AccessCredentials( + accessToken, + refreshToken, + scopes, + idToken: '', ); + auth = buildAuth(credentials: credentialsWithEmptyIdToken); await expectLater( auth.login((_) {}), throwsA( @@ -200,14 +207,14 @@ void main() { }); test('should not set the user when token claims are malformed', () async { - when(() => accessCredentials.idToken).thenReturn( - '''eyJhbGciOiJSUzI1NiIsImN0eSI6IkpXVCJ9.eyJmb28iOiJiYXIifQ.LaR0JfOiDrS1AuABC38kzxpSjRLJ_OtfOkZ8hL6I1GPya-cJYwsmqhi5eMBwEbpYHcJhguG5l56XM6dW8xjdK7JbUN6_53gHBosSnL-Ccf29oW71Ado9sxO17YFQyihyMofJ_v78BPVy2H5O10hNjRn_M0JnnAe0Fvd2VrInlIE''', - ); - auth = Auth( - httpClient: httpClient, - obtainAccessCredentials: - (clientId, scopes, client, userPrompt) async => accessCredentials, + final credentialsWithMalformedIdToken = AccessCredentials( + accessToken, + refreshToken, + scopes, + idToken: + '''eyJhbGciOiJSUzI1NiIsImN0eSI6IkpXVCJ9.eyJmb28iOiJiYXIifQ.LaR0JfOiDrS1AuABC38kzxpSjRLJ_OtfOkZ8hL6I1GPya-cJYwsmqhi5eMBwEbpYHcJhguG5l56XM6dW8xjdK7JbUN6_53gHBosSnL-Ccf29oW71Ado9sxO17YFQyihyMofJ_v78BPVy2H5O10hNjRn_M0JnnAe0Fvd2VrInlIE''', ); + auth = buildAuth(credentials: credentialsWithMalformedIdToken); await expectLater( auth.login((_) {}), throwsA( @@ -232,8 +239,8 @@ void main() { auth.logout(); expect(auth.email, isNull); expect(auth.isAuthenticated, isFalse); - expect(Auth().email, isNull); - expect(Auth().isAuthenticated, isFalse); + expect(buildAuth().email, isNull); + expect(buildAuth().isAuthenticated, isFalse); }); }); diff --git a/packages/shorebird_cli/test/src/commands/build_command_test.dart b/packages/shorebird_cli/test/src/commands/build_command_test.dart index acf2e4f4a..8673b82fa 100644 --- a/packages/shorebird_cli/test/src/commands/build_command_test.dart +++ b/packages/shorebird_cli/test/src/commands/build_command_test.dart @@ -6,7 +6,6 @@ import 'package:mason_logger/mason_logger.dart'; import 'package:mocktail/mocktail.dart'; import 'package:shorebird_cli/src/auth/auth.dart'; import 'package:shorebird_cli/src/commands/build_command.dart'; -import 'package:shorebird_cli/src/config/config.dart'; import 'package:shorebird_cli/src/validators/validators.dart'; import 'package:shorebird_code_push_client/shorebird_code_push_client.dart'; import 'package:test/test.dart'; @@ -31,7 +30,6 @@ class _MockShorebirdFlutterValidator extends Mock void main() { group('build', () { late ArgResults argResults; - late Directory applicationConfigHome; late http.Client httpClient; late Auth auth; late CodePushClient codePushClient; @@ -41,7 +39,6 @@ void main() { late ShorebirdFlutterValidator flutterValidator; setUp(() { - applicationConfigHome = Directory.systemTemp.createTempSync(); argResults = _MockArgResults(); httpClient = _MockHttpClient(); auth = _MockAuth(); @@ -70,7 +67,6 @@ void main() { }, validators: [flutterValidator], )..testArgResults = argResults; - testApplicationConfigHome = (_) => applicationConfigHome.path; when(() => argResults.rest).thenReturn([]); when(() => auth.isAuthenticated).thenReturn(true); diff --git a/packages/shorebird_cli/test/src/commands/login_command_test.dart b/packages/shorebird_cli/test/src/commands/login_command_test.dart index 1f94af463..3694884fa 100644 --- a/packages/shorebird_cli/test/src/commands/login_command_test.dart +++ b/packages/shorebird_cli/test/src/commands/login_command_test.dart @@ -5,7 +5,6 @@ import 'package:mocktail/mocktail.dart'; import 'package:path/path.dart' as p; import 'package:shorebird_cli/src/auth/auth.dart'; import 'package:shorebird_cli/src/commands/login_command.dart'; -import 'package:shorebird_cli/src/config/config.dart'; import 'package:test/test.dart'; class _MockAuth extends Mock implements Auth {} @@ -27,8 +26,6 @@ void main() { auth = _MockAuth(); loginCommand = LoginCommand(auth: auth, logger: logger); - testApplicationConfigHome = (_) => applicationConfigHome.path; - when(() => auth.credentialsFilePath).thenReturn( p.join(applicationConfigHome.path, 'credentials.json'), ); diff --git a/packages/shorebird_cli/test/src/commands/patch_command_test.dart b/packages/shorebird_cli/test/src/commands/patch_command_test.dart index afa2b5309..f39a7ba59 100644 --- a/packages/shorebird_cli/test/src/commands/patch_command_test.dart +++ b/packages/shorebird_cli/test/src/commands/patch_command_test.dart @@ -8,7 +8,6 @@ import 'package:path/path.dart' as p; import 'package:shorebird_cli/src/auth/auth.dart'; import 'package:shorebird_cli/src/cache.dart' show Cache; import 'package:shorebird_cli/src/commands/patch_command.dart'; -import 'package:shorebird_cli/src/config/config.dart'; import 'package:shorebird_cli/src/shorebird_build_mixin.dart'; import 'package:shorebird_cli/src/validators/validators.dart'; import 'package:shorebird_code_push_client/shorebird_code_push_client.dart'; @@ -81,7 +80,6 @@ flutter: - shorebird.yaml'''; late ArgResults argResults; - late Directory applicationConfigHome; late Auth auth; late Progress progress; late Logger logger; @@ -129,7 +127,6 @@ flutter: setUp(() { argResults = _MockArgResults(); - applicationConfigHome = Directory.systemTemp.createTempSync(); auth = _MockAuth(); progress = _MockProgress(); logger = _MockLogger(); @@ -165,7 +162,6 @@ flutter: httpClient: httpClient, validators: [flutterValidator], )..testArgResults = argResults; - testApplicationConfigHome = (_) => applicationConfigHome.path; when(() => argResults.rest).thenReturn([]); when(() => argResults['arch']).thenReturn(arch); diff --git a/packages/shorebird_cli/test/src/commands/release_command_test.dart b/packages/shorebird_cli/test/src/commands/release_command_test.dart index 8962313f9..833b03528 100644 --- a/packages/shorebird_cli/test/src/commands/release_command_test.dart +++ b/packages/shorebird_cli/test/src/commands/release_command_test.dart @@ -7,7 +7,6 @@ import 'package:mocktail/mocktail.dart'; import 'package:path/path.dart' as p; import 'package:shorebird_cli/src/auth/auth.dart'; import 'package:shorebird_cli/src/commands/commands.dart'; -import 'package:shorebird_cli/src/config/config.dart'; import 'package:shorebird_cli/src/shorebird_build_mixin.dart'; import 'package:shorebird_cli/src/validators/validators.dart'; import 'package:shorebird_code_push_client/shorebird_code_push_client.dart'; @@ -65,7 +64,6 @@ flutter: - shorebird.yaml'''; late ArgResults argResults; - late Directory applicationConfigHome; late http.Client httpClient; late Auth auth; late Progress progress; @@ -107,7 +105,6 @@ flutter: setUp(() { argResults = _MockArgResults(); - applicationConfigHome = Directory.systemTemp.createTempSync(); httpClient = _MockHttpClient(); auth = _MockAuth(); progress = _MockProgress(); @@ -137,7 +134,6 @@ flutter: logger: logger, validators: [flutterValidator], )..testArgResults = argResults; - testApplicationConfigHome = (_) => applicationConfigHome.path; when(() => argResults.rest).thenReturn([]); when(() => argResults['arch']).thenReturn(arch); diff --git a/packages/shorebird_cli/test/src/commands/run_command_test.dart b/packages/shorebird_cli/test/src/commands/run_command_test.dart index 94da550ae..d8af4d684 100644 --- a/packages/shorebird_cli/test/src/commands/run_command_test.dart +++ b/packages/shorebird_cli/test/src/commands/run_command_test.dart @@ -8,7 +8,6 @@ import 'package:mason_logger/mason_logger.dart'; import 'package:mocktail/mocktail.dart'; import 'package:shorebird_cli/src/auth/auth.dart'; import 'package:shorebird_cli/src/commands/run_command.dart'; -import 'package:shorebird_cli/src/config/config.dart'; import 'package:shorebird_cli/src/validators/validators.dart'; import 'package:shorebird_code_push_client/shorebird_code_push_client.dart'; import 'package:test/test.dart'; @@ -36,7 +35,6 @@ class _MockShorebirdFlutterValidator extends Mock void main() { group('run', () { late ArgResults argResults; - late Directory applicationConfigHome; late http.Client httpClient; late Auth auth; late Logger logger; @@ -48,7 +46,6 @@ void main() { setUp(() { argResults = _MockArgResults(); - applicationConfigHome = Directory.systemTemp.createTempSync(); httpClient = _MockHttpClient(); auth = _MockAuth(); logger = _MockLogger(); @@ -75,14 +72,13 @@ void main() { ], )..testArgResults = argResults; - testApplicationConfigHome = (_) => applicationConfigHome.path; - when(() => argResults.rest).thenReturn([]); when(() => auth.isAuthenticated).thenReturn(true); when(() => auth.client).thenReturn(httpClient); when(() => logger.progress(any())).thenReturn(_MockProgress()); - when(() => androidInternetPermissionValidator.validate()) - .thenAnswer((_) async => []); + when( + () => androidInternetPermissionValidator.validate(), + ).thenAnswer((_) async => []); when(() => flutterValidator.validate()).thenAnswer((_) async => []); });