Add support for running tests by Arthurm1 · Pull Request #144 · microsoft/build-server-for-gradle

Arthurm1 · 2024-05-24T14:15:43Z

Allows running of tests using buildTarget/test BSP request

There appears to be no JVM specific TestParamsDataKind defined in the BSP spec, only Scala based ones. These should be fine to use on any JVM language (Java, Scala, Kotlin etc.)

The following TestParamsDataKind types are supported...
scala-test which allows the client to run a list of tests at the class level per build target.
scala-test-suites-selection which allows the client to run a list of tests at the method level in a list of classes but only for 1 build target.

There are a number of caveats to testing...

Gradle >= 2.6 is required to run tests
Gradle >= 3.5 is required if environment variables are passed
TaskStartParams and TaskFinishParams do not contain a Location as Gradle only supplies the suite, class, method for each test, not the file uri. That information is then lost as BSP has no way of reporting location except by file uri.
BSP has no structured way of reporting back the stacktrace on failed tests so that is done in the TestFinish#message

plugin/src/main/java/com/microsoft/java/bs/gradle/plugin/SourceSetsModelBuilder.java

server/src/main/java/com/microsoft/java/bs/core/internal/reporter/TestReportReporter.java

Arthurm1 · 2024-05-28T11:59:46Z

@jdneo Added TestStartEx and TestFinishEx

jdneo

I will next try to integrate this into the java test runner extension and see if there is any gaps:

test runner delegates test request to GBS -> GBS reports the test result back -> test runner shows the test result.

This PR will be suspended temporarily during this progress. Thank you @Arthurm1 for your contribution!

server/src/main/java/com/microsoft/java/bs/core/internal/gradle/GradleApiConnector.java

Arthurm1 · 2024-05-31T12:27:09Z

@jdneo I've raised this build-server-protocol/build-server-protocol#674

I've gone for new data types as I don't think suite and class would be applicable to all languages but feel free to comment otherwise

server/src/main/java/com/microsoft/java/bs/core/internal/services/BuildTargetService.java

server/src/main/java/com/microsoft/java/bs/core/internal/reporter/TestReportReporter.java

jdneo

Thank you @Arthurm1, I'll continue the integration work between this work and the java test runner extension.

BTW, I guess we need to update this proposal: build-server-protocol/build-server-protocol#674 as well?

server/src/main/java/com/microsoft/java/bs/core/internal/reporter/TestReportReporter.java

Arthurm1 · 2024-06-17T13:52:30Z

Thank you @Arthurm1, I'll continue the integration work between this work and the java test runner extension.

BTW, I guess we need to update this proposal: build-server-protocol/build-server-protocol#674 as well?

I'll update it when we're completely happy with the structure here.

jdneo · 2024-06-18T02:00:06Z

...c/test/java/com/microsoft/java/bs/core/internal/server/BuildTargetServerIntegrationTest.java

  }

+  @Test
+  void testRunTestsProjectServer() {


Maybe we can consider to separate the test feature tests to a separate file since this is a big one. And I have a feeling that more and more cases will be added for different test framework, and different usage for the same framework.

Happy to move the testing to a different integ test.

server/src/main/java/com/microsoft/java/bs/core/internal/services/BuildTargetService.java

jdneo · 2024-06-19T11:51:28Z

BTW, during the integration, I found an issue about the JUnit 5 @nested support for Gradle Test Launcher: gradle/gradle#29603

Just in case you may have the same problem.

server/src/main/java/com/microsoft/java/bs/core/internal/reporter/TestReportReporter.java

Arthurm1 · 2024-06-22T22:40:25Z

Added TestNG and Spock tests. I've just used whatever is currently sent back for display name etc. I'm not sure if that's what's wanted but it's a start.

jdneo · 2024-06-25T05:00:28Z

Some test cases failed. Except for that, the PR LGTM. One last thing I hope to do is to separate the related test cases to another file.

I can do that as well. @Arthurm1 Let me know if you want me to do that.

Arthurm1 · 2024-06-25T12:08:03Z

@jdneo I can't get the test classes to fail locally. And I can't see why they would fail in the way they have. I've added a commit to test out whether there is an issue with messages appearing on listeners from previous tests. This commit doesn't use the connection cache for tests. I don't intend for it to be merged - I'm just hoping to see if the tests return a different result. Could you run the github test actions on this PR again please

jdneo · 2024-06-26T08:55:57Z

I tried to run test on my mac and windows (without the last commit:
Connector cache test - do not merge). Mac passed. While windows testSpock() failed. Haven't got a chance to look into it.

But if it's related with cache. I think we need to separate the test to more small pieces. Like: one test case only to cover one test class/method, which makes it more maintainable.

jdneo · 2024-06-27T12:14:54Z

Wait... The GitHub Action runs on ubuntu, I thinking it's nothing todo with the Windows and SystemRoot stuff.

I don't know why Github is picking up that change but locally we are not.

Is that because when locally run, the build server picks your gradle local installation(GRADLE_HOME env)? While I believe GitHub does not have that set.

Is that because the gradle version used by java-test? If I add the gradle-wrapper.properties file into it, and point to 8.8. The test will pass. Otherwise, the test fails, where gradle complain that getTestDisaplayName() does not exist.

jdneo · 2024-06-27T12:25:41Z

No it should not related with the gradle version, because github pipeline only fails two cases. If it's due to the version, more cases should fail.

But is that safe to call getTestDisaplayName()? Since it's a new API.

Arthurm1 · 2024-06-27T14:43:14Z

But is that safe to call getTestDisplayName()? Since it's a new API.

I'm not sure. I don't know whether the we can use it because the tooling API is at 8.8 or whether Gradle also needs to be at 8.8.

If I try different versions of Gradle locally I can't get getTestDisplayName to fail.
What happens when you get it to fail - is there a NoSuchMethodError?

jdneo · 2024-06-28T02:58:11Z

It says:

AbstractMethodError@93 "java.lang.AbstractMethodError: Receiver class org.gradle.internal.build.event.types.DefaultTestDescriptor does not define or inherit an implementation of the resolved method 'abstract java.lang.String getTestDisplayName()' of interface org.gradle.tooling.internal.protocol.events.InternalTestDescriptor."

See this commit: 4c01d1c, I forced the test project gradle version fix to 8.7 by adding the wrapper property file. Now more cases failed. Which highly like due to the API getTestDisplayName() I guess.

jdneo · 2024-06-28T02:59:40Z

I think maybe we need to:

rollback the getTestDisplayName() since it looks like now back compatible.
Disable the two failing env tests. I can track it as a debt and solve it in another PR.

Arthurm1 · 2024-06-28T14:49:56Z

I've wrapped getTestDisplayName so it should handle older versions.

Don't know how to disable tests purely on Github - is there an env var?

Arthurm1 · 2024-06-28T15:17:34Z

Added an additional sourceset to test

Arthurm1 · 2024-06-29T09:51:37Z

Here is says that "CI=true" will be set for github actions.

I've disabled the 2 env var tests based on this. Also fixed formatting

jdneo

LGTM

jdneo · 2024-07-01T02:59:08Z

Thank you @Arthurm1 for your contribution.

We can consider to support test debug delegation. Looks like BSP does not have that flag in request body. Maybe another extended point?

Arthurm1 · 2024-07-01T08:59:16Z

@jdneo I think Metals used debugSessionStart but for DebugSessionParamsDataKind used the same data kinds we use for tests e.g. scala-test-suites-selection and scala-test.

What's kind of interesting is that different clients might want different debug adapters....

Metals is primarily for Scala development so it uses https://github.com/scalacenter/scala-debug-adapter
For Kotlin there is https://github.com/fwcd/kotlin-debug-adapter
Java has https://github.com/microsoft/java-debug

I imagine the Java one is good enough for all but it would be handy to be able to specify the type in the BSP command and have the build server kick start the relevant one. Another thing to propose to add to BSP spec.

Co-authored-by: Arthur McGibbon <arthur.mcgibbon@h3im.com> Co-authored-by: Sheng Chen <sheche@microsoft.com>

init named pipe test build Server named pipe connection update named pipe stream solution finish forward message finish merge and named pipe fix - Handle older versions of Gradle (microsoft#149) Co-authored-by: Arthur McGibbon <arthur.mcgibbon@h3im.com> fix - Add JDK 22 compatibility support (microsoft#152) Signed-off-by: Sheng Chen <sheche@microsoft.com> fix - Return the root cause of the message (microsoft#156) feat - Add support for running tests (microsoft#144) Co-authored-by: Arthur McGibbon <arthur.mcgibbon@h3im.com> Co-authored-by: Sheng Chen <sheche@microsoft.com> add Launcher by namedPipe with backwards compatible

jdneo · 2024-07-03T07:19:18Z

Hi @Arthurm1, is the test feature working as expected on your Windows? I tested it today on Windows it's not working unless I remove launcher.setEnvironmentVariables(envVars);. But it's fine on Mac.

Arthurm1 · 2024-07-03T08:06:57Z

@jdneo Yes it works for me running gradlew server:test.

Are you on your cs/fix-test branch? I see you've removed the SystemRoot env var setting on that branch and that was needed for Windows to work.

jdneo · 2024-07-03T08:13:59Z

Ah, I mean the code itself. Not test.

I'm testing the integration with the gradle build server test support feature. Everything works fine one Mac. But on my windows, I have to comment launcher.setEnvironmentVariables(envVars);, otherwise the tooling api won't run the selected tests successfully.

Not sure if it's related with issue: gradle/gradle#29785. Or it's caused by my own environment.

Arthurm1 · 2024-07-03T08:28:20Z

Ah - it works if I add SystemRoot property...

        // Running Gradle tests on Windows seems to require the SystemRoot env var
        // Otherwise Windows complains "Unrecognized Windows Sockets error: 10106"
        String systemRoot = System.getenv("SystemRoot");
        if (systemRoot != null) {
            env.put("SystemRoot", systemRoot);
        }

I should probably add this to GradleAPIConnector#runTests. It's unfortunate that there is no TestLauncher#addExtraEnvVars method

Arthurm1 · 2024-07-03T08:41:14Z

@jdneo see #164

Arthurm1 force-pushed the test_run branch from f0ad798 to a36c589 Compare May 25, 2024 10:22

jdneo reviewed May 27, 2024

View reviewed changes

plugin/src/main/java/com/microsoft/java/bs/gradle/plugin/SourceSetsModelBuilder.java Show resolved Hide resolved

jdneo reviewed May 27, 2024

View reviewed changes

server/src/main/java/com/microsoft/java/bs/core/internal/reporter/TestReportReporter.java Outdated Show resolved Hide resolved

jdneo reviewed May 27, 2024

View reviewed changes

server/src/main/java/com/microsoft/java/bs/core/internal/reporter/TestReportReporter.java Outdated Show resolved Hide resolved

Arthurm1 force-pushed the test_run branch from a36c589 to 687927e Compare May 28, 2024 11:59

jdneo mentioned this pull request May 30, 2024

VS Code does not recognize junit tests microsoft/vscode-java-test#1692

Open

jdneo reviewed May 30, 2024

View reviewed changes

server/src/main/java/com/microsoft/java/bs/core/internal/gradle/GradleApiConnector.java Outdated Show resolved Hide resolved

Add support for running tests

5462e49

Arthurm1 force-pushed the test_run branch from 687927e to 5462e49 Compare May 30, 2024 12:56

jdneo reviewed Jun 4, 2024

View reviewed changes

server/src/main/java/com/microsoft/java/bs/core/internal/services/BuildTargetService.java Show resolved Hide resolved

jdneo reviewed Jun 5, 2024

View reviewed changes

server/src/main/java/com/microsoft/java/bs/core/internal/reporter/TestReportReporter.java Outdated Show resolved Hide resolved

Arthurm1 added 2 commits June 6, 2024 12:47

Add test hierarchy to taskId

4682ea0

Add TestName to preserve test hierarchy

7711c0c

jdneo reviewed Jun 17, 2024

View reviewed changes

server/src/main/java/com/microsoft/java/bs/core/internal/reporter/TestReportReporter.java Show resolved Hide resolved

jdneo reviewed Jun 17, 2024

View reviewed changes

server/src/main/java/com/microsoft/java/bs/core/internal/reporter/TestReportReporter.java Outdated Show resolved Hide resolved

jdneo reviewed Jun 18, 2024

View reviewed changes

server/src/main/java/com/microsoft/java/bs/core/internal/services/BuildTargetService.java Outdated Show resolved Hide resolved

jdneo reviewed Jun 20, 2024

View reviewed changes

server/src/main/java/com/microsoft/java/bs/core/internal/reporter/TestReportReporter.java Outdated Show resolved Hide resolved

Arthurm1 added 2 commits June 20, 2024 09:54

Handle exceptions in @BeforeAll

4b7938c

Add TestNG and Spock tests

3da2823

Don't overwrite testClasses map

46e330a

Connector cache test - do not merge

a5f6953

jdneo added 2 commits June 28, 2024 10:18

test

4c01d1c

test gradle version

0613b26

jdneo force-pushed the test_run branch from d4ca3c1 to 0613b26 Compare June 28, 2024 02:48

Handle versions before getTestDisplayName

b298691

Add additional sourceset test

96b5a64

Fix checkstyle and disable env var tests on CI

a185f1a

Revert the test change

2d13cc3

jdneo added this to the 0.3.0 milestone Jul 1, 2024

jdneo added the enhancement New feature or request label Jul 1, 2024

jdneo approved these changes Jul 1, 2024

View reviewed changes

jdneo merged commit 63c1ea5 into microsoft:develop Jul 1, 2024

jdneo mentioned this pull request Jul 1, 2024

Support debug test. #161

Closed

Arthurm1 deleted the test_run branch July 1, 2024 09:23

Jiaaming pushed a commit to Jiaaming/build-server-for-gradle that referenced this pull request Jul 1, 2024

feat - Add support for running tests (microsoft#144)

d84a543

Co-authored-by: Arthur McGibbon <arthur.mcgibbon@h3im.com> Co-authored-by: Sheng Chen <sheche@microsoft.com>

Conversation

Arthurm1 commented May 24, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Arthurm1 commented May 28, 2024

Uh oh!

jdneo left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Arthurm1 commented May 31, 2024

Uh oh!

Uh oh!

Uh oh!

jdneo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Arthurm1 commented Jun 17, 2024

Uh oh!

jdneo Jun 18, 2024

Choose a reason for hiding this comment

Uh oh!

Arthurm1 Jun 19, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jdneo commented Jun 19, 2024

Uh oh!

Uh oh!

Arthurm1 commented Jun 22, 2024

Uh oh!

jdneo commented Jun 25, 2024

Uh oh!

Arthurm1 commented Jun 25, 2024

Uh oh!

jdneo commented Jun 26, 2024

Uh oh!

jdneo commented Jun 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jdneo commented Jun 27, 2024

Uh oh!

Arthurm1 commented Jun 27, 2024

Uh oh!

jdneo commented Jun 28, 2024

Uh oh!

jdneo commented Jun 28, 2024

Uh oh!

Arthurm1 commented Jun 28, 2024

Uh oh!

Arthurm1 commented Jun 28, 2024

Uh oh!

Arthurm1 commented Jun 29, 2024

Uh oh!

jdneo left a comment

Choose a reason for hiding this comment

Uh oh!

jdneo commented Jul 1, 2024

Uh oh!

Arthurm1 commented Jul 1, 2024

Uh oh!

jdneo commented Jul 3, 2024

Uh oh!

Arthurm1 commented Jul 3, 2024

Uh oh!

jdneo commented Jul 3, 2024

Uh oh!

Arthurm1 commented Jul 3, 2024

Uh oh!

Arthurm1 commented Jul 3, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

jdneo left a comment •

edited

Loading

jdneo commented Jun 27, 2024 •

edited

Loading