-
Notifications
You must be signed in to change notification settings - Fork 55
fix: refactor smoke test tests to use already running gradle instance #1542
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
Conversation
|
A new generated diff is ready to view.
|
|
A new generated diff is ready to view.
|
This comment has been minimized.
This comment has been minimized.
|
A new generated diff is ready to view.
|
This comment has been minimized.
This comment has been minimized.
|
A new generated diff is ready to view.
|
This comment has been minimized.
This comment has been minimized.
| "-Paws.kotlin.native=false", // FIXME: Remove `-Paws.kotlin.native=false` when Kotlin Native is ready | ||
| val processBuilder = | ||
| ProcessBuilder( | ||
| if (System.getProperty("os.name").contains("Windows", ignoreCase = true)) ".\\gradlew.bat" else "./gradlew", |
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.
You should re-use PlatformProvider.System which already offers utilities for this
|
A new generated diff is ready to view.
|
This comment has been minimized.
This comment has been minimized.
|
|
||
| processBuilder.environment().putAll(envVars) | ||
|
|
||
| val buildResult = if (expectingFailure) task.buildAndFail() else task.build() |
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.
Are we no longer asserting that the Gradle task fails? ProcessBuilder should have an option to get the exit error code which could be processed in a similar way to buildAndFail()
|
A new generated diff is ready to view.
|
|
|
A new generated diff is ready to view.
|
Affected ArtifactsNo artifacts changed size |



Issue #
N/A
Description of changes
Uses a
ProcessBuilderinstead of aGradleRunnerto run the tests. Assures already running Gradle instance is used instead of starting up a new one.This should lower memory usage of the tests and brings test run times from ~3 min to ~30 sec
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.