feat: adding gradlew version to the doctor output #2255
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2255 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 186 186
Lines 5862 5877 +15
=========================================
+ Hits 5862 5877 +15
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
packages/shorebird_cli/test/src/commands/doctor_command_test.dart
Outdated
Show resolved
Hide resolved
| @@ -58,9 +58,7 @@ Gradlew get gradlew => read(gradlewRef); | |||
| class Gradlew { | |||
| String get executable => platform.isWindows ? 'gradlew.bat' : 'gradlew'; | |||
|
|
|||
| /// Return the set of product flavors configured for the app at [projectPath]. | |||
| group('when gradlew exists', () { | ||
| group( | ||
| 'when on linux', | ||
| () { | ||
| setUp(() { | ||
| when(() => platform.isLinux).thenReturn(true); | ||
| when(() => platform.isMacOS).thenReturn(false); | ||
| when(() => platform.isWindows).thenReturn(false); | ||
| File( | ||
| p.join(tempDir.path, 'android', 'gradlew'), | ||
| ).createSync(recursive: true); | ||
| }); | ||
|
|
||
| test('returns true', () { | ||
| expect( | ||
| runWithOverrides(() => gradlew.exists(tempDir.path)), | ||
| isTrue, | ||
| ); | ||
| }); | ||
| }, |
There was a problem hiding this comment.
I think we can use testOn for these instead of mocking platform.isOS, now that we're running tests on all supported OSes.
There was a problem hiding this comment.
Also, are the macos tests at all different from the linux tests? Can we have a single test for both?
There was a problem hiding this comment.
Changed to use testOn and kept a single test for macos and linux
| () { | ||
| expect(gradlew.exists(tempDir.path), isTrue); | ||
| }, | ||
| testOn: 'linux || mac-os', |
There was a problem hiding this comment.
Neat, I didn't know OS1 || OS2 was supported syntax
There was a problem hiding this comment.
yeah, me neither, I just learned. Apparently testOn uses this package: https://pub.dev/packages/boolean_selector
| when(() => platform.isLinux).thenReturn(true); | ||
| when(() => platform.isMacOS).thenReturn(false); | ||
| when(() => platform.isWindows).thenReturn(false); |
There was a problem hiding this comment.
This one I am not sure if it is worth to run in all platforms, since all is being tested is the parsing of the output, which shouldn't change (at least the relevant part for us), no matter which platform we are.
And if I change to testOn, then I will need to write one test for each platform (or at least of for unix based and one for windows), because we need to mock the executable (which you can see a couple of lines above) path differently.
Lmk if this makes sense
There was a problem hiding this comment.
You don't need to write one for each platform. I'm just suggesting that we use a consistent method of targeting a platform for a test.
There was a problem hiding this comment.
right, changed to use testOn on all tests.
Description
This PR is a first step in the path to Fix #1821.
It simply adds the version of the gradlew version in
shorebird doctor.In follow up PRs I plan to capture errors when running gradle, check if the error where a
Unsupported class file major version XXone and if so, display a message to user explaining that this caused by the java version, that they can check the used java version and gralde version, and use the gradle compatibilty matrix to check which java version they should use.Type of Change