kotlin-language-server icon indicating copy to clipboard operation
kotlin-language-server copied to clipboard

Failing tests

Open libka-b opened this issue 1 year ago • 21 comments

Hello, running gradlew :server:test leads to 8 failed tests. I also noticed the tests are not run as part of github actions. It would really make the codebase more error prune to run the tests on every commit.

If desirable, I would like to help with fixing the tests and setting up running them on every commit.

libka-b avatar Feb 25 '23 04:02 libka-b

Hello, running gradlew :server:test leads to 8 failed tests.

That does not happen on my machine with several different versions of Java (11, 17, 18). It also doesn't happen in the pipeline.

Have you run clean to clean up any remnants of old code from other branches? We have switched build system DSL to Kotlin recently, so some code has changed a bit. Old remnants in build-directories may be present. I have had many issues with that, especially in Maven and bigger frameworks like Spring. Remnants of various classes that have changed names etc. in a branch. Unsure why it would happen now.

I also noticed the tests are not run as part of github actions. It would really make the codebase more error prune to run the tests on every commit.

The pipeline runs the goals :server:build :shared:build, which includes running tests (given that they are not already run). You can verify this by either cloning a fresh copy like the Actions does, or by first doing a clean. You will notice when running build locally that you get statuses from :server:test and similar (this is during run, after running for a while they will obviously have different numbers and tests):

> :server:test > 4 tests completed, 1 skipped
> :server:test > Executing test org.javacs.kt.ClassPathTest

(example-build in pipeline where you can verify that :server:test is run as part of it)

So is the issue that you were missing an explicit :server:test declaration in the pipeline? Or is there something I'm missing here?

If desirable, I would like to help with fixing the tests and setting up running them on every commit.

They already run on every commit given that it is inside a pull request. You can probably set up Github Actions running on your fork as well if you think that is not enough 🙂

themkat avatar Feb 25 '23 06:02 themkat

I see. Didn't realize build also runs test, in that case it's probably enough.

Anyway, the tests are failing even after cleaning up the workspace and, at least in some cases, it seems to be because I'm running them on windows machine (github actions run them on ubuntu). See this failing test for example:

org.javacs.kt.GradleDSLScriptTest > edit repositories FAILED
    java.lang.AssertionError:
    Expected: a collection containing "repositories"
         but: was "C__Users_lbaka_Documents_projects_kotlin_kotlin_language_server_server_build_resources_test_kotlinDSLWorkspace_settings_gradle", was "C__Users_lbaka_Documents_projects_kotlin_kotlin_language_server_server_build_resources_test_kotlinDSLWorkspace_settings_gradle"
        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.GradleDSLScriptTest.edit repositories(GradleDSLScriptTest.kt:13)

It looks like platform dependent paths are handled incorrectly (or maybe wrong assumptions are taken, I didn't take a deep look into it), which may also affect the language server's function on windows machines.

libka-b avatar Feb 25 '23 20:02 libka-b

@themkat if I run the build task on an M1 mac, I get the following failures. Not sure about other systems because I haven't tried but the builds on GitHub actions pass just fine using the same version of the repo. Someone on the discord channel had mentioned the same issue and I haven't dug into it yet.

2:52:23 pm: Executing 'build'...

Starting Gradle Daemon...
Gradle Daemon started in 1 s 575 ms
> Task :compileKotlin NO-SOURCE
> Task :compileJava NO-SOURCE
> Task :processResources NO-SOURCE
> Task :classes UP-TO-DATE
> Task :jar
> Task :inspectClassesForKotlinIC
> Task :assemble
> Task :compileTestKotlin NO-SOURCE
> Task :compileTestJava NO-SOURCE
> Task :processTestResources NO-SOURCE
> Task :testClasses UP-TO-DATE
> Task :test NO-SOURCE
> Task :check UP-TO-DATE
> Task :build
> Task :shared:processResources
> Task :server:processResources NO-SOURCE
> Task :server:copyPropertiesToDSLTestWorkspace
> Task :server:copyPropertiesToTestWorkspace
> Task :server:processTestResources
> Task :shared:processTestResources NO-SOURCE
> Task :shared:compileKotlin
> Task :shared:compileJava NO-SOURCE
> Task :shared:classes
> Task :shared:jar
> Task :shared:inspectClassesForKotlinIC
> Task :shared:assemble
> Task :shared:compileTestKotlin
> Task :shared:compileTestJava NO-SOURCE
> Task :shared:testClasses UP-TO-DATE
> Task :shared:test
> Task :shared:check
> Task :shared:build
> Task :server:compileKotlin
> Task :server:compileJava NO-SOURCE
> Task :server:classes UP-TO-DATE
> Task :server:jar
> Task :server:inspectClassesForKotlinIC
> Task :server:startScripts
> Task :server:distTar
> Task :server:distZip
> Task :server:assemble

> Task :server:compileTestKotlin
w: file:///Users/henryfraser/repos/kotlin-language-server/server/src/test/kotlin/org/javacs/kt/LanguageServerTestFixture.kt:131:20 'constructor TextDocumentContentChangeEvent(Range!, Int!, String!)' is deprecated. Deprecated in Java
w: file:///Users/henryfraser/repos/kotlin-language-server/server/src/test/kotlin/org/javacs/kt/SimpleScriptTest.kt:46:13 Variable 'resultValue' is never used

> Task :server:compileTestJava NO-SOURCE
> Task :server:testClasses

> Task :server:test

org.javacs.kt.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)

org.javacs.kt.ClassPathTest > find gradle classpath FAILED
    java.lang.AssertionError: 
    Expected: a collection containing a string containing "junit"
         but: was "ClassPathEntry(compiledJar=/Users/henryfraser/.m2/repository/org/jetbrains/kotlin/kotlin-stdlib/1.8.0/kotlin-stdlib-1.8.0.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)

141 tests completed, 2 failed, 3 skipped

> Task :server:test FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':server:test'.
> There were failing tests. See the report at: file:///Users/henryfraser/repos/kotlin-language-server/server/build/reports/tests/test/index.html

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.

* Get more help at https://help.gradle.org

BUILD FAILED in 1m 59s
19 actionable tasks: 19 executed
2:54:25 pm: Execution finished 'build'.

RenFraser avatar Feb 27 '23 04:02 RenFraser

@themkat if I run the build task on an M1 mac, I get the following failures. Not sure about other systems because I haven't tried but the builds on GitHub actions pass just fine using the same version of the repo. Someone on the discord channel had mentioned the same issue and I haven't dug into it yet.

2:52:23 pm: Executing 'build'...

Starting Gradle Daemon...
Gradle Daemon started in 1 s 575 ms
> Task :compileKotlin NO-SOURCE
> Task :compileJava NO-SOURCE
> Task :processResources NO-SOURCE
> Task :classes UP-TO-DATE
> Task :jar
> Task :inspectClassesForKotlinIC
> Task :assemble
> Task :compileTestKotlin NO-SOURCE
> Task :compileTestJava NO-SOURCE
> Task :processTestResources NO-SOURCE
> Task :testClasses UP-TO-DATE
> Task :test NO-SOURCE
> Task :check UP-TO-DATE
> Task :build
> Task :shared:processResources
> Task :server:processResources NO-SOURCE
> Task :server:copyPropertiesToDSLTestWorkspace
> Task :server:copyPropertiesToTestWorkspace
> Task :server:processTestResources
> Task :shared:processTestResources NO-SOURCE
> Task :shared:compileKotlin
> Task :shared:compileJava NO-SOURCE
> Task :shared:classes
> Task :shared:jar
> Task :shared:inspectClassesForKotlinIC
> Task :shared:assemble
> Task :shared:compileTestKotlin
> Task :shared:compileTestJava NO-SOURCE
> Task :shared:testClasses UP-TO-DATE
> Task :shared:test
> Task :shared:check
> Task :shared:build
> Task :server:compileKotlin
> Task :server:compileJava NO-SOURCE
> Task :server:classes UP-TO-DATE
> Task :server:jar
> Task :server:inspectClassesForKotlinIC
> Task :server:startScripts
> Task :server:distTar
> Task :server:distZip
> Task :server:assemble

> Task :server:compileTestKotlin
w: file:///Users/henryfraser/repos/kotlin-language-server/server/src/test/kotlin/org/javacs/kt/LanguageServerTestFixture.kt:131:20 'constructor TextDocumentContentChangeEvent(Range!, Int!, String!)' is deprecated. Deprecated in Java
w: file:///Users/henryfraser/repos/kotlin-language-server/server/src/test/kotlin/org/javacs/kt/SimpleScriptTest.kt:46:13 Variable 'resultValue' is never used

> Task :server:compileTestJava NO-SOURCE
> Task :server:testClasses

> Task :server:test

org.javacs.kt.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)

org.javacs.kt.ClassPathTest > find gradle classpath FAILED
    java.lang.AssertionError: 
    Expected: a collection containing a string containing "junit"
         but: was "ClassPathEntry(compiledJar=/Users/henryfraser/.m2/repository/org/jetbrains/kotlin/kotlin-stdlib/1.8.0/kotlin-stdlib-1.8.0.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)

141 tests completed, 2 failed, 3 skipped

> Task :server:test FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':server:test'.
> There were failing tests. See the report at: file:///Users/henryfraser/repos/kotlin-language-server/server/build/reports/tests/test/index.html

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.

* Get more help at https://help.gradle.org

BUILD FAILED in 1m 59s
19 actionable tasks: 19 executed
2:54:25 pm: Execution finished 'build'.

That is indeed weird, as I'm also using an M1 Mac as my main machine and have done so for almost 2 years now. Never had any issues with the tests or codebase 😱 If you have switched branches there may be remnants of other code interfering. Does these failure happen even on a clean repo? (minimum: gradlew clean, or a full reset of the repo with git clean -fdx).

themkat avatar Feb 27 '23 17:02 themkat

I wonder if it's some sort of environment set up then? Do all the tests pass if you do a gradle and git clean and re-run the tests @themkat?

Looking at the test junit should be on classpath:

I've stepped through it and it looks like it gets to the typeHoverAt function and the hover text returns null. This is because the hoverText variable is null. In my debugger, I can see that the scope variable contains a CODE_BLOCK which seems ok? The result of renderTypeOf(expression, file.bindingContextOf(expression, scope)) returns null. It's testing the MainWorkspaceFile.kt I'm not too familiar with the logic so I'm not quite sure what's expected and what's not for some of these calls just yet

RenFraser avatar Feb 27 '23 20:02 RenFraser

I wonder if it's some sort of environment set up then? Do all the tests pass if you do a gradle and git clean and re-run the tests @themkat?

Yes. I only need to do a gradle clean when I switch branches. Everything has just worked so far. Never needed the git clean in this particular repo (though I have needed it in some Maven projects for similar issues), but it can be useful if you have remnants of files that are not tracked by git.

Looking at the test junit should be on classpath:

I've stepped through it and it looks like it gets to the typeHoverAt function and the hover text returns null. This is because the hoverText variable is null. In my debugger, I can see that the scope variable contains a CODE_BLOCK which seems ok? The result of renderTypeOf(expression, file.bindingContextOf(expression, scope)) returns null. It's testing the MainWorkspaceFile.kt I'm not too familiar with the logic so I'm not quite sure what's expected and what's not for some of these calls just yet

I would have to look further into that tomorrow. It seems weird that it should be different on our machines 😱 I see that it also runs okay in the pipeline introduced in #431 for Mac OS X (even though those are probably still x86?).

themkat avatar Feb 27 '23 20:02 themkat

Sounds great, thanks!

RenFraser avatar Feb 27 '23 21:02 RenFraser

Btw. Could what is the Java version you use? Is it Temurin/AdoptOpenJDK, or something else? And which version number? I have not had any luck in finding possible issues, as it seems to work for me with any version of Java I throw at it :( Both native M1 Java 17, and my x86 Java 11 run through Rosetta 2 (have no clue why I do it, but beyond this project I mostly use Java 19, Temurin)

themkat avatar Mar 01 '23 18:03 themkat

No problem. I'm using Corretto 11 (I tend to try to stick to Corretto versions because that's what we use for work)

RenFraser avatar Mar 01 '23 19:03 RenFraser

Java 19 HotSpot. In my case the test junit should be on classpath fails because fileId starts with file:// which is not valid path on windows (it works in browsers though). Here's a reference, it's for .NET, but works in general. I tried changing the code to val fileId = TextDocumentIdentifier(file.toString()) which crashes with

java.net.URISyntaxException: Illegal character in opaque part at index 2: C:\...

libka-b avatar Mar 02 '23 04:03 libka-b

I've tested it on ubuntu by cloning the repo and running ./gradlew build on ubuntu. Being that it works on the pipelines for windows/mac/ubuntu and not locally on either macOS or ubuntu for me, I think it might be something to do with an environment setup. I'll have to take a look at the build script in both Gradle and the build GitHub action and see if I can spot anything out of the ordinary on my machine

RenFraser avatar Mar 04 '23 03:03 RenFraser

A quick test to use java Temurin 11 via IntelliJ doesn't seem to have an effect either.

RenFraser avatar Mar 05 '23 01:03 RenFraser

Have looked more into this, and I still can't find anything. Neither my home Mac M1, work Mac (x86), various docker containers (for all intents and purposes Linux), can reproduce any test failures. They behave exactly the same as our workflows 😨 Only thing I haven't tried yet is doing anything in IntelliJ, but I would rather avoid installing that bloatware....

Do any of you have any special JAVA_OPTIONS settings (it is an environment variable), or other settings on your computers that might affect the JVM running stuff? Or any special settings related to Java or Kotlin in IntelliJ? Remember IntellIJ having a shitton of settings back in the day (I interacted more with people using it before, due to having to help them with it at work).

themkat avatar Mar 07 '23 18:03 themkat

Ok I've had a go on EC2 using ubuntu 22.04 using Temurin 11. I've run ./gradlew build with the following output:

Starting a Gradle Daemon, 2 incompatible and 3 stopped Daemons could not be reused, use --status for details

> Task :server:compileKotlin
'compileJava' task (current target is 19) and 'compileKotlin' task (current target is 11) jvm target compatibility should be set to the same Java version.
By default will become an error since Gradle 8.0+! Read more: https://kotl.in/gradle/jvm/target-validation
Consider using JVM toolchain: https://kotl.in/gradle/jvm/toolchain


> Task :server:compileTestKotlin
'compileTestJava' task (current target is 19) and 'compileTestKotlin' task (current target is 11) jvm target compatibility should be set to the same Java version.
By default will become an error since Gradle 8.0+! Read more: https://kotl.in/gradle/jvm/target-validation
Consider using JVM toolchain: https://kotl.in/gradle/jvm/toolchain

w: file:///home/ubuntu/kotlin-language-server/server/src/test/kotlin/org/javacs/kt/LanguageServerTestFixture.kt:131:20 'constructor TextDocumentContentChangeEvent(Range!, Int!, String!)' is deprecated. Deprecated in Java
w: file:///home/ubuntu/kotlin-language-server/server/src/test/kotlin/org/javacs/kt/SimpleScriptTest.kt:46:13 Variable 'resultValue' is never used

> Task :server:test

org.javacs.kt.OverrideMemberTest > should find members in jdk object FAILED
    java.lang.AssertionError:
    Expected: iterable over ["override fun equals(other: Any?): Boolean { }", "override fun hashCode(): Int { }", "override fun toString(): String { }", "override fun run() { }", "override fun clone(): Any { }", "override fun start() { }", "override fun interrupt() { }", "override fun isInterrupted(): Boolean { }", "override fun countStackFrames(): Int { }", "override fun getContextClassLoader(): ClassLoader { }", "override fun setContextClassLoader(cl: ClassLoader) { }", "override fun getStackTrace(): (Array<(StackTraceElement..StackTraceElement?)>..Array<out (StackTraceElement..StackTraceElement?)>) { }", "override fun getId(): Long { }", "override fun getState(): State { }", "override fun getUncaughtExceptionHandler(): UncaughtExceptionHandler { }", "override fun setUncaughtExceptionHandler(eh: UncaughtExceptionHandler) { }"] in any order
         but: Not matched: "override fun inheritExtentLocalBindings(container: ThreadContainer) { }"
        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.OverrideMemberTest.should find members in jdk object(OverrideMemberTest.kt:109)

141 tests completed, 1 failed, 3 skipped

> Task :server:test FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':server:test'.
> There were failing tests. See the report at: file:///home/ubuntu/kotlin-language-server/server/build/reports/tests/test/index.html

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.

* Get more help at https://help.gradle.org

BUILD FAILED in 3m 16s
28 actionable tasks: 5 executed, 23 up-to-date

Using Maven from homebrew:

When I run mvn --version:

Maven home: /home/linuxbrew/.linuxbrew/Cellar/maven/3.9.1/libexec
Java version: 19.0.2, vendor: Homebrew, runtime: /home/linuxbrew/.linuxbrew/Cellar/openjdk/19.0.2/libexec
Default locale: en, platform encoding: UTF-8
OS name: "linux", version: "5.15.0-1028-aws", arch: "amd64", family: "unix"

I'm not sure what the difference is between this environment and what the GitHub actions would be? I think that you might be right @themkat in that there's something going on with IntelliJ but I want to get a working repro somewhere other than my local environment so that I can narrow down what the cause may be.

RenFraser avatar Mar 26 '23 00:03 RenFraser

Could you provide the rough steps that you used in your environments? Maybe if I could use the docker environment I could get a working build locally and work backwards from that

RenFraser avatar Mar 26 '23 00:03 RenFraser

The test above is probably failing due to using a new version of Java (the test prints current target is 19, so doesn't seem like Temurin 11). I have never heard about the method inheritExtentLocalBindings for starters, so my guess is that it is introduced in a newer version of the JDK. Tested my local Temurin Java 19 again now (recently updated it with brew upgrade), and I get the same error as you. Have to build with Java 17 for it to work... Weird that the error did not materialize pre-upgrade (as it was simply adding 0.2 or something to the JDK version). So we probably need to look into if this method should be included in the result of override methods, or if it is simply a bug we need to fix. I notice that it is package private in the Thread-class now: https://github.com/openjdk/jdk19/blob/fb27ddcbe5a503ddd841e55baaa9a10f8422b736/src/java.base/share/classes/java/lang/Thread.java#L295 (There is probably more Java 19 upgrade work coming for us in the future... My guess is that we probably build and support LTS versions, and that Java 21 will be the next version we support)

Container that runs the test without issues, debian:bullseye-slim, run commands:

apt-get update
apt-get install -y openjdk-17-jdk git maven
git clone https://github.com/fwcd/kotlin-language-server.git
cd kotlin-language-server
./gradlew build

A little worrying that different setup, sub-versions, environments etc. can get such different results 🙁

themkat avatar Mar 26 '23 07:03 themkat

Apologies for not responding @themkat. I'd still had no luck trying to narrow it down. I've synced my fork of the repo and pulled it locally. It looks like @fwcd's latest changes have fixed the test issues for me. I'm not sure which change it is but I suspect that it'd be the update to Gradle 8.1. I guess we can mark this as closed unless we intend to keep it open to root cause it. What do you think?

RenFraser avatar Apr 15 '23 11:04 RenFraser

I think some of the test failures that one might encounter locally were due to using newer JDKs than supported by the test workspace Gradle causing these tests to silently fail the Gradle dependency resolution.

fwcd avatar Apr 15 '23 11:04 fwcd

Chiming in here as I also had a test in :server:test failing (find maven classpath). Though I believe my issue was caused by something different to above.

java.lang.AssertionError: 
Expected: a collection containing a string containing "junit"
     but: was "ClassPathEntry(compiledJar=/Users/calluml/.sdkman/candidates/kotlin/1.8.20/lib/kotlin-stdlib.jar, sourceJar=null)"

In the end I worked that it was due to the order of Maven repository discovery.

I use SDKMAN! (script v5.18.1, native v0.2.2) on an Intel Mac, and have let that manage my JVM setup. I only seem to have an environment variable set for MAVEN_HOME, which points to /Users/calluml/.sdkman/candidates/maven/current. But SDKMAN! has put my Maven repository at $HOME/.m2.

Because the language server prioritises MAVEN_HOME over $HOME/.m2 for the Maven repository location, the classpath resolving fails.

I am fairly new to Java/Kotlin, but is the current prioritisation of discovery sensible? e.g. MAVEN_REPOSITORY > MAVEN_HOME > M2_HOME > $HOME/.m2? It seems correct that MAVEN_REPOSITORY and M2_HOME take precedence over $HOME/.m2. But should MAVEN_HOME also take precedence? Or should SDKMAN! have set a value for MAVEN_REPOSITORY?

If this issue occurs for everyone who uses SDKMAN! (which is a popular tool), then it would be useful to fix (happy to open an MR on whatever the best solution is). Either:

  • Check a /repository directory exists in the locations before setting it as the mavenRepository
  • Reorder some of the repository discovery prioritisation
  • Update documentation to point out this effect (e.g. ask users to add a MAVEN_REPOSITORY to their .bashrc).

calamont avatar Oct 06 '23 06:10 calamont

Yeah, the dependency resolution in the (non-Gradle-workspace) tests is unfortunately quite dependent on the build environment. Currently it uses whichever Kotlin you have installed on your system as opposed to the Gradle-managed Kotlin dependency that the language server is built with. A potential solution would be to perform all tests with the Kotlin libraries provided by the test workspace.

Regarding the Maven priority: I do not use Maven enough to have an informed opinion on that[^1]. Preferring the user repository over the Maven installation (if that is what MAVEN_HOME usually points to) feels reasonable though, so feel free to open a PR.

[^1]: Googling a bit shows that M2_HOME seems to be deprecated anyway and shouldn't be used anymore, though I couldn't find anything further on MAVEN_HOME.

fwcd avatar Oct 06 '23 12:10 fwcd