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

Bump Kotlin to 2.0.0 #576

Closed
wants to merge 4 commits into from
Closed

Bump Kotlin to 2.0.0 #576

wants to merge 4 commits into from

Conversation

AlexandrosAlexiou
Copy link
Contributor

@AlexandrosAlexiou AlexandrosAlexiou commented Jun 2, 2024

  • Bump Kotlin version to 2.0.0 to the related Gradle files

@github-actions github-actions bot added dependency resolution Related to the project dependency/standard library resolver code quality Refactoring, tests etc. build Related to the language server's own build (including Gradle updates etc.) labels Jun 2, 2024
@AlexandrosAlexiou
Copy link
Contributor Author

AlexandrosAlexiou commented Jun 2, 2024

This PR is an attempt to bump the kotlin version to 2.0.0 and ultimately start making myself familiar with the codebase as I would be interested in supporting the project.

That being said, I am noticing an issue with theseAdditionalWorkspaceTest.kt and ClassPathTest.kt test class methods not passing right now, but I created the PR anyway to spark the discussion.

There seems to be an issue resolving junit in the classpath, so hovering over the assertTrue method returns null.

I packaged the server after the update and instantiated it from Neovim, and it seems that it works, but the hovering over assertTrue returns null.

Any thought or pointers on where to start looking to resolve this issue?

Tests output:

AdditionalWorkspaceTest > junit should be on classpath FAILED
    java.lang.AssertionError:
    Expected: a string containing "fun assertTrue"
         but: was "```kotlin
    null
    ```"
        at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
        at org.junit.Assert.assertThat(Assert.java:865)
        at org.junit.Assert.assertThat(Assert.java:832)
        at org.javacs.kt.AdditionalWorkspaceTest.junit should be on classpath(AdditionalWorkspaceTest.kt:31)

ClassPathTest > find gradle classpath FAILED
    java.lang.AssertionError:
    Expected: a collection containing a string containing "junit"
         but: was "ClassPathEntry(compiledJar=~/.m2/repository/org/jetbrains/kotlin/kotlin-stdlib/1.9.22/kotlin-stdlib-1.9.22.jar, sourceJar=null)"
        at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
        at org.junit.Assert.assertThat(Assert.java:865)
        at org.junit.Assert.assertThat(Assert.java:832)
        at org.javacs.kt.ClassPathTest.find gradle classpath(ClassPathTest.kt:28)

@fwcd
Copy link
Owner

fwcd commented Jun 2, 2024

Thanks for looking into this, very much appreciated! It would be nice if you could split out the refactorings/cleanups into smaller commits for easier reviewability.

Regarding the failing test, you could try running ./gradlew test with --info to get output. Looks like it can't find JUnit for some reason, a wild guess would be that you'd have to bump the Kotlin version in the test workspace too (under server/src/test/resources/additionalWorkspace)

@AlexandrosAlexiou
Copy link
Contributor Author

AlexandrosAlexiou commented Jun 2, 2024

Thanks for looking into this, very much appreciated! It would be nice if you could split out the refactorings/cleanups into smaller commits for easier reviewability.

Regarding the failing test, you could try running ./gradlew test with --info to get output. Looks like it can't find JUnit for some reason, a wild guess would be that you'd have to bump the Kotlin version in the test workspace too (under server/src/test/resources/additionalWorkspace)

Hey! Thanks for the reply. I do have bumped it here.

I removed all other changes for now that are not related with the version bump.
I will continue taking a look why we cant find junit. Any help would be much appreciated!

@AlexandrosAlexiou
Copy link
Contributor Author

AlexandrosAlexiou commented Jun 8, 2024

Hello, turns out it was an issue that I still do not understand, but now all the tests are passing.
Please check so that we can move forward with this MR.

Tested also in my Neovim setup:
Screenshot 2024-06-08 at 1 23 12 PM

@AlexandrosAlexiou
Copy link
Contributor Author

@fwcd Sorry for pinging you, can you take a look when you have some time?

@fwcd fwcd added this to the 1.4.0 milestone Jul 28, 2024
@fwcd fwcd force-pushed the main branch 2 times, most recently from de3d15c to b490bce Compare July 28, 2024 22:54
@AlexandrosAlexiou
Copy link
Contributor Author

AlexandrosAlexiou commented Aug 8, 2024

Closing this PR, opened a new one to bump version to 2.0.10.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Related to the language server's own build (including Gradle updates etc.) code quality Refactoring, tests etc. dependency resolution Related to the project dependency/standard library resolver
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants