Skip to content
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

Java path for kotlin tests compilation on Windows machines #412

Open
wants to merge 2 commits into
base: development
Choose a base branch
from

Conversation

SergeyDatskiv
Copy link
Collaborator

@SergeyDatskiv SergeyDatskiv commented Nov 4, 2024

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 kotlinc requires 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 JavaTestCompiler to the KotlinTestCompiler class. This required changing the TestCompilerFacotry to find the Java path and pass it to KotlinTestCompiler. 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 KotlinTestCompiler now gets a javaHomeDirectoryPath which is not directly assigned in the constructor because that would require changing the TestCompiler class. I thought that TestCompiler class 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

  • Please test that nothing broke on Mac with regard to Java- and Kotlin-based test case generation. Thanks!

Why is merge request needed

Closes #410

  • I have checked that I am merging into correct branch

@SergeyDatskiv SergeyDatskiv added the bug Something isn't working label Nov 4, 2024
@SergeyDatskiv SergeyDatskiv self-assigned this Nov 4, 2024
@SergeyDatskiv SergeyDatskiv added the Ready for review PR redy for review label Nov 6, 2024
@Hello-zoka
Copy link
Collaborator

Probably closes #320

@Vladislav0Art
Copy link
Collaborator

Kotlin compilation modulo this issue works on my Windows now.

Copy link
Collaborator

@Vladislav0Art Vladislav0Art left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with minor suggestions. There is a question about the Java home path param in the Kotlin compiler.

I checked both Java and Kotlin test generation on Mac. Both work as expected.

Removed some todos, decreased line length and declared variables in a constructor.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Ready for review PR redy for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kotlinc requires javac on Windows
3 participants