Export `MILL_TEST_RESOURCE_DIR` to BSP project definitions #5265

arturaz · 2025-06-06T15:32:22Z

We've introduced allForkEnv to JavaModule, which now contains the environment variables that Mill adds, in addition to user-defined environment variables.

Updated the integration tests to include the newly-exported environment variables so that ./mill integration.ide[bsp-server].local.__.testForked mill.integration.BspServerTests.requestSnapshots would work.

Tested with Metals (requires a fix in Metals: scalameta/metals#7544) and Idea (only works with Junit for now: https://youtrack.jetbrains.com/issue/SCL-23964/Running-utest-test-in-Intellij-does-not-pass-environment-variables-configured-in-Mill)

arturaz · 2025-06-06T15:34:38Z

integration/ide/bsp-server/resources/snapshots/build-targets-jvm-run-environments.json

+        "uri": "file:///workspace/app/test"
+      },
+      "classpath": [
+        "/coursier-cache/https/repo1.maven.org/maven2/ch/qos/logback/logback-core/1.5.15/logback-core-1.5.15.jar",


This is questionable, why do classpath entries here don't have file:// prefix?

Tricky case. The BSP spec often uses URI[], but here a string[] is required. I still think using an URI-string would be more consistent.

E.g. in JavaOptionsItem is also a classpath: string[] specified, but we return URI-strings.

But why does it differ between the app and test projects?

Don't know. Looks like a bug.

arturaz · 2025-06-06T15:35:24Z

integration/ide/bsp-server/resources/snapshots/build-targets-jvm-run-environments.json

+      "jvmOptions": [],
+      "workingDirectory": "/workspace",
+      "environmentVariables": {
+        "MILL_TEST_RESOURCE_DIR": "/workspace/app/test/resources"


Should this be exported here, or only in jvm-test-environments.json?

The test runner is a dedicated runner, hence trait TestModule extends RunModule. forkEnv and forkArgs come from RunModule, so it's correct that these are also exposed here. It's a Mill-specific redundancy, since we always use dedicated test modules in contrast to sbt, Maven and Gradle, where tests are some feature/scope of normal modules.

lefou · 2025-06-07T09:27:32Z

integration/ide/bsp-server/resources/snapshots/build-targets-jvm-test-environments.json

+        {
+          "className": "mill.testrunner.entrypoint.TestRunnerMain",
+          "arguments": [
+            "file:/coursier-cache/https/repo1.maven.org/maven2/org/scala-lang/scala3-library_3/<scala3-version>/scala3-library_3-<scala3-version>.jar,file:/coursier-cache/https/repo1.maven.org/maven2/com/lihaoyi/mill-moduledefs_3/0.11.4/mill-moduledefs_3-0.11.4.jar,file:/coursier-cache/https/repo1.maven.org/maven2/org/scala-lang/modules/scala-xml_3/2.3.0/scala-xml_3-2.3.0.jar,file:/coursier-cache/https/repo1.maven.org/maven2/org/scala-lang/scala-reflect/2.13.15/scala-reflect-2.13.15.jar,file:/coursier-cache/https/repo1.maven.org/maven2/com/lumidion/sonatype-central-client-requests_3/0.3.0/sonatype-central-client-requests_3-0.3.0.jar,file:/coursier-cache/https/repo1.maven.org/maven2/io/get-coursier/coursier_2.13/2.1.25-M13/coursier_2.13-2.1.25-M13.jar,file:/coursier-cache/https/repo1.maven.org/maven2/io/get-coursier/coursier-core_2.13/2.1.25-M13/coursier-core_2.13-2.1.25-M13.jar,file:/coursier-cache/https/repo1.maven.org/maven2/io/get-coursier/coursier-cache_2.13/2.1.25-M13/coursier-cache_2.13-2.1.25-M13.jar,file:/coursier-cache/https/repo1.maven.org/maven2/io/get-coursier/coursier-util_2.13/2.1.25-M13/coursier-util_2.13-2.1.25-M13.jar,file:/coursier-cache/https/repo1.maven.org/maven2/io/get-coursier/versions_2.13/0.5.1/versions_2.13-0.5.1.jar,file:/coursier-cache/https/repo1.maven.org/maven2/io/get-coursier/coursier-jvm_2.13/2.1.25-M13/coursier-jvm_2.13-2.1.25-M13.jar,file:/coursier-cache/https/repo1.maven.org/maven2/org/jline/jline/3.28.0/jline-3.28.0.jar,file:/coursier-cache/https/repo1.maven.org/maven2/com/lihaoyi/mainargs_3/0.7.6/mainargs_3-0.7.6.jar,file:/coursier-cache/https/repo1.maven.org/maven2/com/lihaoyi/requests_3/0.9.0/requests_3-0.9.0.jar,file:/coursier-cache/https/repo1.maven.org/maven2/org/scala-lang/scala-library/<scala-version>/scala-library-<scala-version>.jar,file:/coursier-cache/https/repo1.maven.org/maven2/com/lihaoyi/sourcecode_3/0.4.3-M5/sourcecode_3-0.4.3-M5.jar,file:/coursier-cache/https/repo1.maven.org/maven2/com/lumidion/sonatype-central-client-core_3/0.3.0/sonatype-central-client-core_3-0.3.0.jar,file:/coursier-cache/https/repo1.maven.org/maven2/com/lumidion/sonatype-central-client-upickle_3/0.3.0/sonatype-central-client-upickle_3-0.3.0.jar,file:/coursier-cache/https/repo1.maven.org/maven2/org/scala-sbt/test-interface/1.0/test-interface-1.0.jar,file:/coursier-cache/https/repo1.maven.org/maven2/com/lihaoyi/os-lib_3/0.11.5-M9/os-lib_3-0.11.5-M9.jar,file:/coursier-cache/https/repo1.maven.org/maven2/com/lihaoyi/upickle_3/4.2.1/upickle_3-4.2.1.jar,file:/coursier-cache/https/repo1.maven.org/maven2/com/lihaoyi/upickle-implicits-named-tuples_3/4.2.1/upickle-implicits-named-tuples_3-4.2.1.jar,file:/coursier-cache/https/repo1.maven.org/maven2/com/lihaoyi/pprint_3/0.9.0/pprint_3-0.9.0.jar,file:/coursier-cache/https/repo1.maven.org/maven2/com/lihaoyi/fansi_3/0.5.0/fansi_3-0.5.0.jar,file:/coursier-cache/https/repo1.maven.org/maven2/com/lihaoyi/fastparse_3/3.1.1/fastparse_3-3.1.1.jar,file:/coursier-cache/https/repo1.maven.org/maven2/org/scala-lang/modules/scala-collection-compat_3/2.12.0/scala-collection-compat_3-2.12.0.jar,file:/coursier-cache/https/repo1.maven.org/maven2/com/lihaoyi/geny_3/1.1.1/geny_3-1.1.1.jar,file:/coursier-cache/https/repo1.maven.org/maven2/com/github/plokhotnyuk/jsoniter-scala/jsoniter-scala-core_2.13/2.13.39/jsoniter-scala-core_2.13-2.13.39.jar,file:/coursier-cache/https/repo1.maven.org/maven2/io/get-coursier/coursier-env_2.13/2.1.25-M13/coursier-env_2.13-2.1.25-M13.jar,file:/coursier-cache/https/repo1.maven.org/maven2/io/get-coursier/coursier-exec/2.1.25-M13/coursier-exec-2.1.25-M13.jar,file:/coursier-cache/https/repo1.maven.org/maven2/com/lihaoyi/os-zip/0.11.5-M9/os-zip-0.11.5-M9.jar,file:/coursier-cache/https/repo1.maven.org/maven2/com/lihaoyi/ujson_3/4.2.1/ujson_3-4.2.1.jar,file:/coursier-cache/https/repo1.maven.org/maven2/com/lihaoyi/upack_3/4.2.1/upack_3-4.2.1.jar,file:/coursier-cache/https/repo1.maven.org/maven2/com/lihaoyi/upickle-implicits_3/4.2.1/upickle-implicits_3-4.2.1.jar,file:/coursier-cache/https/repo1.maven.org/maven2/io/get-coursier/dependency_2.13/0.3.2/dependency_2.13-0.3.2.jar,file:/coursier-cache/https/repo1.maven.org/maven2/io/get-coursier/coursier-proxy-setup/2.1.25-M13/coursier-proxy-setup-2.1.25-M13.jar,file:/coursier-cache/https/repo1.maven.org/maven2/org/scala-lang/modules/scala-collection-compat_2.13/2.13.0/scala-collection-compat_2.13-2.13.0.jar,file:/coursier-cache/https/repo1.maven.org/maven2/io/get-coursier/jniutils/windows-jni-utils/0.3.3/windows-jni-utils-0.3.3.jar,file:/coursier-cache/https/repo1.maven.org/maven2/net/java/dev/jna/jna/5.16.0/jna-5.16.0.jar,file:/coursier-cache/https/repo1.maven.org/maven2/com/lihaoyi/upickle-core_3/4.2.1/upickle-core_3-4.2.1.jar,file:/coursier-cache/https/repo1.maven.org/maven2/io/github/alexarchambault/concurrent-reference-hash-map/1.1.0/concurrent-reference-hash-map-1.1.0.jar,file:/coursier-cache/https/repo1.maven.org/maven2/org/scala-lang/modules/scala-xml_2.13/2.3.0/scala-xml_2.13-2.3.0.jar,file:/coursier-cache/https/repo1.maven.org/maven2/io/github/alexarchambault/is-terminal/0.1.2/is-terminal-0.1.2.jar,file:/coursier-cache/https/repo1.maven.org/maven2/org/codehaus/plexus/plexus-archiver/4.10.0/plexus-archiver-4.10.0.jar,file:/coursier-cache/https/repo1.maven.org/maven2/org/codehaus/plexus/plexus-container-default/2.1.1/plexus-container-default-2.1.1.jar,file:/coursier-cache/https/repo1.maven.org/maven2/org/virtuslab/scala-cli/config_2.13/1.1.3/config_2.13-1.1.3.jar,file:/coursier-cache/https/repo1.maven.org/maven2/org/apache/tika/tika-core/3.1.0/tika-core-3.1.0.jar,file:/coursier-cache/https/repo1.maven.org/maven2/io/github/alexarchambault/windows-ansi/windows-ansi/0.0.6/windows-ansi-0.0.6.jar,file:/coursier-cache/https/repo1.maven.org/maven2/com/github/luben/zstd-jni/1.5.7-3/zstd-jni-1.5.7-3.jar,file:/coursier-cache/https/repo1.maven.org/maven2/io/get-coursier/cache-util/2.1.25-M13/cache-util-2.1.25-M13.jar,file:/coursier-cache/https/repo1.maven.org/maven2/javax/inject/javax.inject/1/javax.inject-1.jar,file:/coursier-cache/https/repo1.maven.org/maven2/org/codehaus/plexus/plexus-utils/4.0.1/plexus-utils-4.0.1.jar,file:/coursier-cache/https/repo1.maven.org/maven2/org/codehaus/plexus/plexus-io/3.5.0/plexus-io-3.5.0.jar,file:/coursier-cache/https/repo1.maven.org/maven2/commons-io/commons-io/2.18.0/commons-io-2.18.0.jar,file:/coursier-cache/https/repo1.maven.org/maven2/org/apache/commons/commons-compress/1.27.1/commons-compress-1.27.1.jar,file:/coursier-cache/https/repo1.maven.org/maven2/org/slf4j/slf4j-api/2.0.16/slf4j-api-2.0.16.jar,file:/coursier-cache/https/repo1.maven.org/maven2/io/airlift/aircompressor/0.27/aircompressor-0.27.jar,file:/coursier-cache/https/repo1.maven.org/maven2/org/tukaani/xz/1.9/xz-1.9.jar,file:/coursier-cache/https/repo1.maven.org/maven2/org/codehaus/plexus/plexus-classworlds/2.6.0/plexus-classworlds-2.6.0.jar,file:/coursier-cache/https/repo1.maven.org/maven2/org/apache/xbean/xbean-reflect/3.7/xbean-reflect-3.7.jar,file:/coursier-cache/https/repo1.maven.org/maven2/org/virtuslab/scala-cli/specification-level_2.13/1.1.3/specification-level_2.13-1.1.3.jar,file:/coursier-cache/https/repo1.maven.org/maven2/org/fusesource/jansi/jansi/2.4.1/jansi-2.4.1.jar,file:/coursier-cache/https/repo1.maven.org/maven2/commons-codec/commons-codec/1.17.1/commons-codec-1.17.1.jar,file:/coursier-cache/https/repo1.maven.org/maven2/org/apache/commons/commons-lang3/3.16.0/commons-lang3-3.16.0.jar,file:/mill-workspace/out/core/coursierutil/compile.dest/classes/,file:/mill-workspace/out/core/define/compile.dest/classes/,file:/mill-workspace/out/core/constants/buildInfoResources.dest/,file:/mill-workspace/out/core/constants/compile.dest/classes/,file:/mill-workspace/out/core/api/buildInfoResources.dest/,file:/mill-workspace/out/core/api/compile.dest/classes/,file:/mill-workspace/out/core/util/buildInfoResources.dest/,file:/mill-workspace/out/core/util/compile.dest/classes/,file:/mill-workspace/out/libs/scalalib/api/buildInfoResources.dest/,file:/mill-workspace/out/libs/scalalib/api/compile.dest/classes/,file:/mill-workspace/out/libs/testrunner/entrypoint/compile.dest/classes/,file:/mill-workspace/out/libs/testrunner/compile.dest/classes/,file:/mill-workspace/out/libs/scalalib/compile.dest/classes/,file:/mill-workspace/libs/scalalib/resources/",


Regarding the single slash after file:: Could be a missing sanitizeUri call, that should ensure the same formatting as everywhere else.

…Env` method

…est-resource-dir

lihaoyi · 2025-06-09T08:15:28Z

CC @alexarchambault if you could take a look, since you're familiar with the BSP parts of the codebase

And pass it as URIs to BSP

alexarchambault · 2025-06-09T11:18:14Z

Just pushed a few commits that fix some confusion there was around BSP run and test environments, I think (that was there in main already), and makes sure we pass the class path as URIs for test environments

core/api/src/mill/api/internal/README.md

lihaoyi · 2025-06-09T14:28:19Z

@arturaz looks like some tests need to be updated but otherwise the PR is probably in good shape

lihaoyi · 2025-06-09T14:29:29Z

No run env for test modules, no test env for run modules

@alexarchambault Is this correct? Right now Mill JavaTests or ScalaTests modules all can have public static void main(String[] args) methods , and if they are defined the module can be run from the command line.

lefou · 2025-06-10T15:13:44Z

What is the rationale for adding allForkEnv? Is there an issue with overriding forkEnv?

It is a bit misleading. We have various all-prefixed tasks in Mill, but they typically don't introduce new data but just aggregate other tasks. Here, we use the all prefix to provide some new stuff. Why can't we provide the env variables by just overriding forEnv?

lefou · 2025-06-10T15:18:45Z

@lihaoyi

No run env for test modules, no test env for run modules

@alexarchambault Is this correct? Right now Mill JavaTests or ScalaTests modules all can have public static void main(String[] args) methods , and if they are defined the module can be run from the command line.

Don't know where the first quote comes from, but I agree with @lihaoyi here in that test modules should always provide a run environment. This is a consequence of TestModule being a RunModule.

lihaoyi · 2025-06-10T15:49:09Z

What is the rationale for adding allForkEnv? Is there an issue with overriding forkEnv?

It is a bit misleading. We have various all-prefixed tasks in Mill, but they typically don't introduce new data but just aggregate other tasks. Here, we use the all prefix to provide some new stuff. Why can't we provide the env variables by just overriding forEnv?

The basic goal is the same as the other all tasks: to avoid people accidentally overriding forkEnv and forgetting to call super, which would result in confusing misbehaviors when the "builtin" environment variables are missing. This is similar to ivyDeps and mandatoryIvyDeps being combined into allIvyDeps, but without a mandatoryForkEnv since it's just included directly in allForkEnv

alexarchambault · 2025-06-10T16:21:52Z

No run env for test modules, no test env for run modules

@alexarchambault Is this correct? Right now Mill JavaTests or ScalaTests modules all can have public static void main(String[] args) methods , and if they are defined the module can be run from the command line.

It feels there was some duplication. jvmRunEnvironment and jvmTestEnvironment were both returning the same thing. With this commit, we only return actual main classes in jvmRunEnvironment, and commands to run tests in jvmTestEnvironment.

Do we support the scenario you're describing (main methods in test modules) already in BSP? It seems some more tweaks are needed for that. I could add a test case, and have a look.

alexarchambault · 2025-06-10T16:26:35Z

@lihaoyi

No run env for test modules, no test env for run modules

@alexarchambault Is this correct? Right now Mill JavaTests or ScalaTests modules all can have public static void main(String[] args) methods , and if they are defined the module can be run from the command line.

Don't know where the first quote comes from, but I agree with @lihaoyi here in that test modules should always provide a run environment. This is a consequence of TestModule being a RunModule.

My changes don't split modules between RunModules on the one hand, and test ones on the other. For the jvmRunEnvironment request, it keeps RunModules (which should include TestModules), using discovered main classes. For jvmTestEnvironment, it looks for TestModules, and uses their test environment.

But the code around here currently prevents TestModules from being seen as pure RunModules (by matching them as TestModule first, and retaining only their test environment), and using their discovered main classes. That might need fixing (and a test case).

alexarchambault · 2025-06-10T20:32:32Z

I pushed my commits, alongside a fix for (what I think is) an issue and that I discussed above, in #5287

…est-resource-dir

alexarchambault · 2025-06-11T13:44:13Z

The additional snapshot data build-targets-jvm-test-environments.json embeds parts of the Mill class path, whose versions should change quite often. #5287 addresses that. Maybe it should be merged before the PR here (I can take care of pushing a merge commit to disentangle things here if needed).

lihaoyi · 2025-06-11T14:19:04Z

@alexarchambault go it, I'll merge this first and we can do the other PR as a follow up

integration/ide/bsp-server/resources/snapshots/build-targets-jvm-test-environments.json

integration/ide/bsp-server/src/BspServerTestUtil.scala

This is a follow-up of some changes added in #5265. This further changes how we handle `buildTarget/jvmRunEnvironment` and `buildTarget/jvmTestEnvironment` requests, by making sure main classes in test modules are taken into account in `buildTarget/jvmRunEnvironment`, and by cleaning-up some test snapshot data of `buildTarget/jvmTestEnvironment`, so that they don't change too often and are more maintainable.

…#5292) The current way of changing the code and then changing it back is error prone, as witnessed in #5265 (comment) . This changes it to use an environment variable, which has no chance of being committed to the codebase. Pull request: #5292 Co-authored-by: Tobias Roeser <[email protected]>

arturaz added 2 commits June 6, 2025 17:47

Export MILL_TEST_RESOURCE_DIR to BSP project

ce35816

Ignore Gradle generated files

b46a32c

arturaz commented Jun 6, 2025

View reviewed changes

lefou reviewed Jun 7, 2025

View reviewed changes

arturaz added 3 commits June 9, 2025 09:58

refactor: centralize environment variable management through `allFork…

9a7d22d

…Env` method

Merge remote-tracking branch 'upstream/main' into fix/5039-fix-mill-t…

206e838

…est-resource-dir

Fix typo.

fa9399b

arturaz changed the title ~~Draft: Export MILL_TEST_RESOURCE_DIR to BSP project definitions~~ Export MILL_TEST_RESOURCE_DIR to BSP project definitions Jun 9, 2025

unused import

4c168ef

alexarchambault added 4 commits June 9, 2025 12:47

Merge branch 'main' into fix/5039-fix-mill-test-resource-dir

1bb4ede

No run env for test modules, no test env for run modules

379aea9

Type test class path as Seq[Path] to pass it around

c7852ac

And pass it as URIs to BSP

fixup No run env for test modules, no test env for run modules

218ea6d

lihaoyi reviewed Jun 9, 2025

View reviewed changes

core/api/src/mill/api/internal/README.md Outdated Show resolved Hide resolved

alexarchambault mentioned this pull request Jun 10, 2025

Tweak BSP JVM run / test environment handling #5287

Merged

arturaz and others added 5 commits June 11, 2025 09:27

Merge remote-tracking branch 'upstream/main' into fix/5039-fix-mill-t…

3f4fbaa

…est-resource-dir

Update the tests to use allForkEnv

48aa172

Move the documentation.

a67c0d2

Merge remote-tracking branch 'upstream/main' into fix/5039-fix-mill-t…

025e58e

…est-resource-dir

fix-test

b42869d

lihaoyi added 3 commits June 11, 2025 18:38

fix-test

9b35317

fix-test

094962b

fix-test

77c2aa5

lihaoyi merged commit 8781d1e into com-lihaoyi:main Jun 11, 2025
38 checks passed

lefou reviewed Jun 11, 2025

View reviewed changes

integration/ide/bsp-server/resources/snapshots/build-targets-jvm-test-environments.json Outdated Show resolved Hide resolved

lefou reviewed Jun 11, 2025

View reviewed changes

integration/ide/bsp-server/src/BspServerTestUtil.scala Outdated Show resolved Hide resolved

lefou added the post-review-required A merged PR which has still open comments or questions, which need to be clarified label Jun 11, 2025

lefou added this to the 1.0.0-RC2 milestone Jun 11, 2025

lefou removed the post-review-required A merged PR which has still open comments or questions, which need to be clarified label Jun 11, 2025

arturaz mentioned this pull request Jun 11, 2025

Update BSP test snapshots via MILL_TESTS_BSP_UPDATE_SNAPSHOTS env var #5292

Merged

Uh oh!

Export MILL_TEST_RESOURCE_DIR to BSP project definitions #5265

Export MILL_TEST_RESOURCE_DIR to BSP project definitions #5265

Uh oh!

Conversation

arturaz commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arturaz Jun 6, 2025

Choose a reason for hiding this comment

Uh oh!

lefou Jun 7, 2025

Choose a reason for hiding this comment

Uh oh!

lefou Jun 7, 2025 • 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!

arturaz Jun 7, 2025

Choose a reason for hiding this comment

Uh oh!

lefou Jun 8, 2025

Choose a reason for hiding this comment

Uh oh!

arturaz Jun 6, 2025

Choose a reason for hiding this comment

Uh oh!

lefou Jun 7, 2025 • 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!

lefou Jun 7, 2025 • 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!

lihaoyi commented Jun 9, 2025

Uh oh!

alexarchambault commented Jun 9, 2025

Uh oh!

Uh oh!

lihaoyi commented Jun 9, 2025

Uh oh!

lihaoyi commented Jun 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lefou commented Jun 10, 2025

Uh oh!

lefou commented Jun 10, 2025

Uh oh!

lihaoyi commented Jun 10, 2025

Uh oh!

alexarchambault commented Jun 10, 2025

Uh oh!

alexarchambault commented Jun 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexarchambault commented Jun 10, 2025

Uh oh!

alexarchambault commented Jun 11, 2025

Uh oh!

Uh oh!

lihaoyi commented Jun 11, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Export `MILL_TEST_RESOURCE_DIR` to BSP project definitions #5265

Export `MILL_TEST_RESOURCE_DIR` to BSP project definitions #5265

arturaz commented Jun 6, 2025 •

edited

Loading

lefou Jun 7, 2025 •

edited

Loading

lefou Jun 7, 2025 •

edited

Loading

lefou Jun 7, 2025 •

edited

Loading

lihaoyi commented Jun 9, 2025 •

edited

Loading

alexarchambault commented Jun 10, 2025 •

edited

Loading