Java path for kotlin tests compilation on Windows machines #412
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
SergeyDatskiv
commented
Nov 4, 2024
src/main/kotlin/org/jetbrains/research/testspark/tools/factories/TestCompilerFactory.kt
Show resolved
Hide resolved
|
Probably closes #320 |
core/src/main/kotlin/org/jetbrains/research/testspark/core/test/kotlin/KotlinTestCompiler.kt
Show resolved
Hide resolved
|
Kotlin compilation modulo this issue works on my Windows now. |
Vladislav0Art
approved these changes
Nov 11, 2024
core/src/main/kotlin/org/jetbrains/research/testspark/core/test/kotlin/KotlinTestCompiler.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/org/jetbrains/research/testspark/tools/factories/TestCompilerFactory.kt
Outdated
Show resolved
Hide resolved
Removed some todos, decreased line length and declared variables in a constructor.
|
This will create a revert commit, are we okay with that?
Do I need to manually check that issue 320 is also fixed? Because Iurii mentioned that it "probably closes" the issue. Thanks! |
Oh! I think we can do it better: when you start git rebase, type # rebase's todo list opened via `rebase --edit-todo`
[some other commits above]
- 0fab360 Merge branch 'development' into SergeyDatskiv/bugfix/410-javac-path-...
+ break (at the end)
Yeah, I think it requires a manual check. E.g., assign |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In this merge request we are closing an issue where Windows users who did not have java in their PATH variable could not run the generated kotlin tests because
kotlincrequires Java to be in the path variable.Description of changes made
To address this issue, I added the same Java Path, which is used by the
JavaTestCompilerto theKotlinTestCompilerclass. This required changing theTestCompilerFacotryto find the Java path and pass it toKotlinTestCompiler. I decided to remove dedicated functions for creating Kotlin and Java test compilers because I thought that now they are more similar than different. Instead, I added a private function for finding the Java home path.The
KotlinTestCompilernow gets ajavaHomeDirectoryPathwhich is not directly assigned in the constructor because that would require changing theTestCompilerclass. I thought thatTestCompilerclass should not be dedicated to the Java / Kotlin language, hence I did not change it.The Java path is set only if we detect that the system is a Windows machine. It is set through a chain of commands via command prompt.
Other notes
Why is merge request needed
Closes #410