Improve test coverage of remote worker files

[fmeum]

Adds a runfiles tree to the input and also runs the test with path mapping enabled.

[Copilot]

Pull Request Overview

This PR aims to improve test coverage of the remote worker functionality by adding a runfiles tree input and enabling tests with conditional path mapping. Key changes include:

• Parameterizing the test method with a boolean flag (enablePathMapping) to control path mapping behavior.
• Adding a runfiles tree setup for tool artifacts and updating assertions to include directories.
• Updating persistent worker key hash values based on the enablePathMapping parameter.

Comments suppressed due to low confidence (2)

src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java:2335

• [nitpick] Consider renaming 'toolDat' to a more descriptive name such as 'toolData' to improve clarity.

Artifact toolDat = ActionsTestUtil.createArtifact(artifactRoot, "tool.dat");

src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java:2367

• [nitpick] The persistent worker key hash literals are duplicated across the test; consider extracting them as constants to reduce potential inconsistencies and improve maintainability.

enablePathMapping ? "0d8336db089ad9f8754f8c551c6c38430e506becfa06508aeebcfbc4530a6c69" : "b218f80e9b457a2b84a130e75266bda866876f9b6b1ca576334ee0fe7961abac"

[fmeum]

@tjgq I pushed a new commit to make the runfiles layout more realistic, which is needed for the test to keep passing with my planned changes.

[fmeum]

I resolved the conflict.

[tjgq]

Is it possible to compute these hashes from the respective paths? In particular, we need to take into account the product name (outputs paths will be blaze-out/... instead of bazel-out/... when we run these tests internally)

[tjgq]

Actually, nevermind; it doesn't look like the paths in this test contain the product name (the tests are failing internally but it must be some other reason - let me take a look)

[tjgq]

Oh, it's the TestConstants.WORKSPACE_NAME (it's google3 instead of _main internally).

I guess there isn't a better way to do this than switch on the workspace name, since computing the persistentWorkerKey using the production logic defeats the point of the test?

[fmeum]

I rewrote the test to avoid 1) any hardcoded digests and 2) access to non-root directories in the Merkle tree, which will help preserve the test as is when I change the Merkle tree computation.

[Copilot]

Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.

Files not reviewed (1)

• src/test/java/com/google/devtools/build/lib/remote/BUILD: Language not supported

Comments suppressed due to low confidence (2)

src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java:2337

• [nitpick] Consider renaming 'toolDat' to 'toolData' for improved clarity.

Artifact toolDat = ActionsTestUtil.createArtifact(artifactRoot, "tool.dat");

src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java:2330

• Consider adding explicit test cases for when 'enablePathMapping' is true and false to ensure both scenarios are adequately covered.

public void buildRemoteActionForRemotePersistentWorkers(@TestParameter boolean enablePathMapping) throws Exception {