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

Java editor shows compile errors for Hamcrest/JUnit references in Java tests #43

Closed
plaird opened this issue Dec 20, 2019 · 7 comments
Closed
Assignees
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@plaird
Copy link
Contributor

plaird commented Dec 20, 2019

Repro:

  1. Create a java_test rule in a BUILD file with a valid junit test

  2. do NOT add hamcrest as a dep

  3. add this line to the test method:

    org.hamcrest.core.StringContains.containsString("test");

  4. bazel test //... (your test should succeed)

  5. in BEF, open the test .java file in the editor, and the hamcrest line will be red squigglied

There are some implicit deps that the command line test runner adds to the classpath, that BEF needs to imply for packages that contain java_test targets. Figure out what they are, and add them.

@plaird plaird added bug Something isn't working good first issue Good for newcomers labels Dec 20, 2019
@plaird plaird self-assigned this Jan 2, 2020
@plaird
Copy link
Contributor Author

plaird commented Jan 2, 2020

This is caused by a known issue in Bazel. The issue can be 'fixed' by setting this in your .bazelrc and running bazel test in your command line build:

test --explicit_java_test_deps=true

Setting that property eliminates implicit deps, and thus requires hamcrest and junit to be explicitly included in the BUILD file where needed. When you do this, there is an explicit dependency on those libs, and so BEF knows to add it to the project classpath in Eclipse. Then JDT will not show red squigglies in the editor.

This is where the Bazel code has that option:

https://github.com/bazelbuild/bazel/blob/master/src/main/java/com/google/devtools/build/lib/rules/java/JavaOptions.java#L275

I didn't track down exactly where those deps are added, but the flag meanders through this code path:

https://github.com/bazelbuild/bazel/blob/master/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java#L454
https://github.com/bazelbuild/bazel/blob/master/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java#L302
https://github.com/bazelbuild/bazel/blob/master/src/main/java/com/google/devtools/build/lib/rules/java/JavaSemantics.java#L332
https://github.com/bazelbuild/bazel/blob/master/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaRuleClasses.java#L351

@plaird
Copy link
Contributor Author

plaird commented Jan 2, 2020

I have decided that I am going to document this as a known limitation of package import, and not do anything automatic in BEF to correct this configuration. There are workarounds below.

How would BEF know what version of junit or hamcrest libs to add? I feel like Bazel is doing the wrong thing here, and I don't want to build BEF logic to accommodate it. Hopefully explicit_java_test_deps will be set to true by default in the future.

Workaround #1: Fix your BUILD files
I just did this for our internal monorepo, and it takes a little bit of time but I think it is the right thing to do anyway. Add 'test --explicit_java_test_deps=true' to your .bazelrc and fix all of the junit/hamcrest dep issues that arise from a 'bazel test //...'

Workaround #2: Manually fix up your classpath in Eclipse after import
For each affected project, do the following:

  1. right click on the Eclipse project in the Package Explorer, and click Build Path -> Configure Build Path
  2. Click on the Libraries tab, and then the Classpath node in the tree
  3. Click Add External Jars... and locate your version of junit/hamcrest on the filesystem

@plaird plaird changed the title Add hamcrest and other implicit dependencies to classpath Java editor shows compile errors for Hamcrest/JUnit references Jan 2, 2020
@plaird plaird changed the title Java editor shows compile errors for Hamcrest/JUnit references Java editor shows compile errors for Hamcrest/JUnit references in Java tests Jan 2, 2020
@plaird
Copy link
Contributor Author

plaird commented Jan 2, 2020

I just requested that we switch the default to disable implicit deps on Bazel discuss:
https://groups.google.com/forum/#!topic/bazel-discuss/COJvRi2W1ok

If that happens, then this becomes a short-term issue and I just doc it like suggested above, or provide a hacky short term solution until people transition to Bazel 3.0. If it looks like implicit deps will remain the default long term, we will need to figure out how to determine which version of these deps to add to each project that needs them.

@plaird
Copy link
Contributor Author

plaird commented Jan 4, 2020

If we implement a hack solution, the way the IJ plugin solves this is by adding the Runner_deploy.jar (actually, Runner_deploy-ijar.jar) to the classpath which is seen in bazel query as:

@bazel_tools//tools/jdk:TestRunner
@remote_java_tools_windows//:java_tools/Runner_deploy.jar
@remote_java_tools_linux//:java_tools/Runner_deploy.jar
@remote_java_tools_darwin//:java_tools/Runner_deploy.jar

with relative path from bazel-bin:
bazel-bin/external/bazel_tools/tools/jdk/_ijar/TestRunner/external/remote_java_tools_darwin/java_tools/Runner_deploy-ijar.jar

But to be correct, this should only be added if: --explicit_java_test_deps=false

@plaird
Copy link
Contributor Author

plaird commented Jan 14, 2020

I am working on adding the hack, but in the meantime have added doc discouraging implicit deps:
https://github.com/salesforce/bazel-eclipse/blob/master/docs/conforming_java_packages.md#recommended-java-conventions

@plaird
Copy link
Contributor Author

plaird commented Jan 16, 2020

added the hack, will write tests next

@plaird
Copy link
Contributor Author

plaird commented Jan 22, 2020

Over a series of commits, this feature was implemented.

User impact:

  • If your Bazel workspace has the following command option: explicit_java_test_deps=true, BEF will NOT add the implicit test runner jar to the JDT classpath
  • If your Bazel workspace has the following command option: explicit_java_test_deps=false (or unspecified, false is the defaut), BEF will add the implicit test runner jar (Runner_deploy-ijar.jar) to the JDT classpath. This will prevent JDT from adding red squigglies to your test classes where junit/hamcrest classes are referenced.

To see what entries the BEF has supplied to the JDT classpath for a project, do the following:

  • open your imported project's node in the Package Explorer
  • open the Bazel Classpath Container node
  • you will see the list of jars added to the JDT classpath below that node

Implementation Details:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

1 participant