-
Notifications
You must be signed in to change notification settings - Fork 118
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Integration tests on windows #1283
Conversation
Buildscan url for ubuntu-workflow run 333642729 |
Buildscan url for ubuntu-workflow run 333752188 |
Buildscan url for ubuntu-workflow run 333813859 |
Buildscan url for ubuntu-workflow run 333883441 |
Codecov Report
@@ Coverage Diff @@
## master #1283 +/- ##
============================================
- Coverage 79.76% 79.66% -0.11%
Complexity 724 724
============================================
Files 235 235
Lines 4483 4489 +6
Branches 768 771 +3
============================================
Hits 3576 3576
- Misses 509 514 +5
- Partials 398 399 +1 |
Buildscan url for ubuntu-workflow run 333925165 |
Buildscan url for ubuntu-workflow run 333964681 |
Buildscan url for ubuntu-workflow run 333977503 |
Buildscan url for ubuntu-workflow run 334081456 |
Buildscan url for ubuntu-workflow run 334114456 |
Buildscan url for ubuntu-workflow run 334138541 |
Buildscan url for ubuntu-workflow run 334193230 |
Buildscan url for ubuntu-workflow run 334621677 |
Buildscan url for ubuntu-workflow run 334653171 |
Buildscan url for ubuntu-workflow run 334659370 |
Buildscan url for ubuntu-workflow run 334710329 |
} catch (e: IOException) { | ||
throw FlankGeneralError("Error: Failed to read service account credential.\n${e.message}") | ||
}.getOrElse { | ||
if (isWindows) GoogleCredentials.fromStream(defaultCredentialPath.toFile().inputStream()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What will be the error message when flan fails reading credentials on Windows as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably similiar to previous one from ServiceAccountCredentials.getApplicationDefault()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I changed it to throws FlankGeneralException like on non-windows.
@@ -69,18 +68,20 @@ object FtlConstants { | |||
} | |||
|
|||
val defaultCredentialPath: Path by lazy { | |||
Paths.get(System.getProperty("user.home"), ".config/gcloud/application_default_credentials.json") | |||
if (isWindows) Paths.get(System.getenv("HOMEPATH"), ".config/gcloud/application_default_credentials.json") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about
val homePath = if(isWindows) (System.getenv("HOMEPATH") else System.getProperty("user.home")
Paths.get(homePath, ".config/gcloud/application_default_credentials.json")
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed! Thanks!
Buildscan url for ubuntu-workflow run 335608461 |
Buildscan url for ubuntu-workflow run 335688160 |
Fixes #1282
Test Plan
Integration tests on windows should works