From eefb9e8cd67e3da05e7ba2fab60c37bfb83b59e6 Mon Sep 17 00:00:00 2001 From: Bryan Oltman Date: Mon, 16 Oct 2023 17:43:20 -0400 Subject: [PATCH] fix(shorebird_cli): prefer Android Studio java over JAVA_HOME (#1405) --- .../lib/src/executables/java.dart | 21 +- .../test/src/executables/java_test.dart | 277 ++++++++++++------ packages/shorebird_cli/test/src/mocks.dart | 4 + 3 files changed, 200 insertions(+), 102 deletions(-) diff --git a/packages/shorebird_cli/lib/src/executables/java.dart b/packages/shorebird_cli/lib/src/executables/java.dart index 392f4e17..6e7ac134 100644 --- a/packages/shorebird_cli/lib/src/executables/java.dart +++ b/packages/shorebird_cli/lib/src/executables/java.dart @@ -4,6 +4,7 @@ import 'package:collection/collection.dart'; import 'package:path/path.dart' as p; import 'package:scoped/scoped.dart'; import 'package:shorebird_cli/src/android_studio.dart'; +import 'package:shorebird_cli/src/os/os.dart'; import 'package:shorebird_cli/src/platform.dart'; /// A reference to a [Java] instance. @@ -23,13 +24,21 @@ class Java { return p.join(javaHome, 'bin', 'java.exe'); } - /// Returns the JAVA_HOME environment variable if set. - /// Otherwise, returns the location where the Android Studio JDK/JRE is installed. - String? get home { - if (platform.environment.containsKey('JAVA_HOME')) { - return platform.environment['JAVA_HOME']; - } + /// Returns the path to the user's JDK, if one is found. + /// + /// Our goal is to match the behavior of the flutter tool. As per the docs at + /// https://github.com/flutter/flutter/blob/stable/packages/flutter_tools/lib/src/android/java.dart#L45-L54, + /// we search for Java in the following places, in order: + /// + /// 1. the runtime environment bundled with Android Studio; + /// 2. the runtime environment found in the JAVA_HOME env variable, if set; or + /// 3. the java binary found on PATH. + String? get home => + _androidStudioJavaPath ?? + platform.environment['JAVA_HOME'] ?? + osInterface.which('java'); + String? get _androidStudioJavaPath { final androidStudioPath = androidStudio.path; if (androidStudioPath == null) return null; if (platform.isMacOS) { diff --git a/packages/shorebird_cli/test/src/executables/java_test.dart b/packages/shorebird_cli/test/src/executables/java_test.dart index 61630d79..540bd931 100644 --- a/packages/shorebird_cli/test/src/executables/java_test.dart +++ b/packages/shorebird_cli/test/src/executables/java_test.dart @@ -6,6 +6,7 @@ import 'package:platform/platform.dart'; import 'package:scoped/scoped.dart'; import 'package:shorebird_cli/src/android_studio.dart'; import 'package:shorebird_cli/src/executables/executables.dart'; +import 'package:shorebird_cli/src/os/os.dart'; import 'package:shorebird_cli/src/platform.dart'; import 'package:test/test.dart'; @@ -14,6 +15,7 @@ import '../mocks.dart'; void main() { group(Java, () { late AndroidStudio androidStudio; + late OperatingSystemInterface osInterface; late Platform platform; late Java java; @@ -22,6 +24,7 @@ void main() { () => body(), values: { androidStudioRef.overrideWith(() => androidStudio), + osInterfaceRef.overrideWith(() => osInterface), platformRef.overrideWith(() => platform), }, ); @@ -35,120 +38,202 @@ void main() { setUp(() { androidStudio = MockAndroidStudio(); + osInterface = MockOperatingSystemInterface(); platform = MockPlatform(); java = Java(); + + when(() => platform.environment).thenReturn({}); + when(() => platform.isWindows).thenReturn(false); + when(() => platform.isMacOS).thenReturn(false); + when(() => platform.isLinux).thenReturn(false); + + when(() => osInterface.which(any())).thenReturn(null); }); group('executable', () { - test('returns correct executable on windows', () async { + group('when on Windows', () { const javaHome = r'C:\Program Files\Java\jdk-11.0.1'; - when(() => platform.isWindows).thenReturn(true); - when(() => platform.environment).thenReturn({'JAVA_HOME': javaHome}); - expect( - runWithOverrides(() => java.executable), - equals(p.join(javaHome, 'bin', 'java.exe')), - ); + + setUp(() { + when(() => platform.isWindows).thenReturn(true); + when(() => platform.environment).thenReturn({'JAVA_HOME': javaHome}); + }); + + test('returns correct executable on windows', () async { + expect( + runWithOverrides(() => java.executable), + equals(p.join(javaHome, 'bin', 'java.exe')), + ); + }); }); - test('returns correct executable on non-windows', () async { - when(() => platform.isWindows).thenReturn(false); - expect( - runWithOverrides(() => java.executable), - equals('java'), - ); + group('when on a non-Windows OS', () { + setUp(() { + when(() => platform.isWindows).thenReturn(false); + }); + + test('returns correct executable on non-windows', () async { + expect( + runWithOverrides(() => java.executable), + equals('java'), + ); + }); }); }); group('home', () { - test('returns existing JAVA_HOME if already set', () async { - const javaHome = r'C:\Program Files\Java\jdk-11.0.1'; - when(() => platform.environment).thenReturn({'JAVA_HOME': javaHome}); - expect( - runWithOverrides(() => java.home), - equals(javaHome), - ); - }); + group('when Android Studio is installed', () { + late Directory jbrDir; - test( - 'returns null if JAVA_HOME is not set and' - ' Android Studio is not installed', () async { - when(() => platform.environment).thenReturn({}); - when(() => platform.isMacOS).thenReturn(false); - when(() => platform.isWindows).thenReturn(false); - when(() => platform.isLinux).thenReturn(false); - expect(runWithOverrides(() => java.home), isNull); - }); + group('when on macOS', () { + setUp(() { + when(() => platform.isMacOS).thenReturn(true); - test('returns correct path on windows', () async { - final tempDir = setUpAppTempDir(); - final androidStudioDir = Directory( - p.join(tempDir.path, 'Android', 'Android Studio'), - )..createSync(recursive: true); - when(() => androidStudio.path).thenReturn(androidStudioDir.path); - final jbrDir = Directory(p.join(androidStudioDir.path, 'jbr')) - ..createSync(); - File( - p.join(tempDir.path, 'android', 'gradlew.bat'), - ).createSync(recursive: true); - when(() => platform.isWindows).thenReturn(true); - when(() => platform.isMacOS).thenReturn(false); - when(() => platform.isLinux).thenReturn(false); - when(() => platform.environment).thenReturn({ - 'PROGRAMFILES': tempDir.path, - 'PROGRAMFILES(X86)': tempDir.path, + final tempDir = setUpAppTempDir(); + final androidStudioDir = Directory( + p.join( + tempDir.path, + 'Applications', + 'Android Studio.app', + 'Contents', + ), + )..createSync(recursive: true); + when(() => androidStudio.path).thenReturn(androidStudioDir.path); + jbrDir = Directory( + p.join(androidStudioDir.path, 'jbr', 'Contents', 'Home'), + )..createSync(recursive: true); + File( + p.join(tempDir.path, 'android', 'gradlew'), + ).createSync(recursive: true); + when(() => platform.environment).thenReturn({'HOME': tempDir.path}); + }); + + test('returns correct path', () async { + await expectLater( + runWithOverrides(() => java.home), + equals(jbrDir.path), + ); + }); + + test('does not check JAVA_HOME or PATH', () { + runWithOverrides(() => java.home); + + verifyNever(() => osInterface.which(any())); + verifyNever(() => platform.environment); + }); + }); + + group('when on Windows', () { + late Directory jbrDir; + + setUp(() { + when(() => platform.isWindows).thenReturn(true); + + final tempDir = setUpAppTempDir(); + final androidStudioDir = Directory( + p.join(tempDir.path, 'Android', 'Android Studio'), + )..createSync(recursive: true); + when(() => androidStudio.path).thenReturn(androidStudioDir.path); + jbrDir = Directory(p.join(androidStudioDir.path, 'jbr')) + ..createSync(); + File( + p.join(tempDir.path, 'android', 'gradlew.bat'), + ).createSync(recursive: true); + when(() => platform.environment).thenReturn({ + 'PROGRAMFILES': tempDir.path, + 'PROGRAMFILES(X86)': tempDir.path, + }); + }); + + test('returns correct path', () async { + await expectLater( + runWithOverrides(() => java.home), + equals(jbrDir.path), + ); + }); + + test('does not check JAVA_HOME or PATH', () { + runWithOverrides(() => java.home); + + verifyNever(() => osInterface.which(any())); + verifyNever(() => platform.environment); + }); + }); + + group('when on Linux', () { + setUp(() { + when(() => platform.isLinux).thenReturn(true); + + final tempDir = setUpAppTempDir(); + final androidStudioDir = Directory( + p.join(tempDir.path, '.AndroidStudio'), + )..createSync(recursive: true); + when(() => androidStudio.path).thenReturn(androidStudioDir.path); + jbrDir = Directory(p.join(androidStudioDir.path, 'jbr')) + ..createSync(recursive: true); + File( + p.join(tempDir.path, 'android', 'gradlew'), + ).createSync(recursive: true); + + when(() => platform.environment).thenReturn({'HOME': tempDir.path}); + }); + + test('returns correct path', () async { + await expectLater( + runWithOverrides(() => java.home), + equals(jbrDir.path), + ); + }); }); - await expectLater( - runWithOverrides(() => java.home), - equals(jbrDir.path), - ); }); - test('returns correct path on MacOS', () async { - final tempDir = setUpAppTempDir(); - final androidStudioDir = Directory( - p.join( - tempDir.path, - 'Applications', - 'Android Studio.app', - 'Contents', - ), - )..createSync(recursive: true); - when(() => androidStudio.path).thenReturn(androidStudioDir.path); - final jbrDir = Directory( - p.join(androidStudioDir.path, 'jbr', 'Contents', 'Home'), - )..createSync(recursive: true); - File( - p.join(tempDir.path, 'android', 'gradlew'), - ).createSync(recursive: true); - when(() => platform.isWindows).thenReturn(false); - when(() => platform.isMacOS).thenReturn(true); - when(() => platform.isLinux).thenReturn(false); - when(() => platform.environment).thenReturn({'HOME': tempDir.path}); - await expectLater( - runWithOverrides(() => java.home), - equals(jbrDir.path), - ); - }); + group('when Android Studio is not installed', () { + group('when JAVA_HOME is set', () { + const javaHome = r'C:\Program Files\Java\jdk-11.0.1'; + setUp(() { + when(() => platform.environment) + .thenReturn({'JAVA_HOME': javaHome}); + }); - test('returns correct path on Linux', () async { - final tempDir = setUpAppTempDir(); - final androidStudioDir = Directory( - p.join(tempDir.path, '.AndroidStudio'), - )..createSync(recursive: true); - when(() => androidStudio.path).thenReturn(androidStudioDir.path); - final jbrDir = Directory(p.join(androidStudioDir.path, 'jbr')) - ..createSync(recursive: true); - File( - p.join(tempDir.path, 'android', 'gradlew'), - ).createSync(recursive: true); - when(() => platform.isWindows).thenReturn(false); - when(() => platform.isMacOS).thenReturn(false); - when(() => platform.isLinux).thenReturn(true); - when(() => platform.environment).thenReturn({'HOME': tempDir.path}); - await expectLater( - runWithOverrides(() => java.home), - equals(jbrDir.path), - ); + test('returns value of JAVA_HOME', () { + expect( + runWithOverrides(() => java.home), + equals(javaHome), + ); + }); + + test('does not check PATH', () { + runWithOverrides(() => java.home); + + verifyNever(() => osInterface.which(any())); + }); + }); + + group('when JAVA_HOME is not set', () { + group("when java is on the user's path", () { + const javaPath = '/path/to/java'; + setUp(() { + when(() => osInterface.which('java')).thenReturn(javaPath); + }); + + test('returns path to java', () { + expect( + runWithOverrides(() => java.home), + equals('/path/to/java'), + ); + }); + }); + + group("when java is not on the user's path", () { + setUp(() { + when(() => osInterface.which('java')).thenReturn(null); + }); + + test('returns null', () { + expect(runWithOverrides(() => java.home), isNull); + }); + }); + }); }); }); }); diff --git a/packages/shorebird_cli/test/src/mocks.dart b/packages/shorebird_cli/test/src/mocks.dart index 27e7125e..70d8798f 100644 --- a/packages/shorebird_cli/test/src/mocks.dart +++ b/packages/shorebird_cli/test/src/mocks.dart @@ -17,6 +17,7 @@ import 'package:shorebird_cli/src/code_push_client_wrapper.dart'; import 'package:shorebird_cli/src/config/config.dart'; import 'package:shorebird_cli/src/doctor.dart'; import 'package:shorebird_cli/src/executables/executables.dart'; +import 'package:shorebird_cli/src/os/os.dart'; import 'package:shorebird_cli/src/patch_diff_checker.dart'; import 'package:shorebird_cli/src/process.dart'; import 'package:shorebird_cli/src/shorebird_env.dart'; @@ -78,6 +79,9 @@ class MockJava extends Mock implements Java {} class MockLogger extends Mock implements Logger {} +class MockOperatingSystemInterface extends Mock + implements OperatingSystemInterface {} + class MockPatchDiffChecker extends Mock implements PatchDiffChecker {} class MockPlatform extends Mock implements Platform {}