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

After repository merge IntelliJ is showing some classes missing #1144

Closed
suztomo opened this issue Dec 16, 2022 · 15 comments · Fixed by #1474
Closed

After repository merge IntelliJ is showing some classes missing #1144

suztomo opened this issue Dec 16, 2022 · 15 comments · Fixed by #1474
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@suztomo
Copy link
Member

suztomo commented Dec 16, 2022

After repository merge IntelliJ is showing some classes missing but Maven build succeeds. Investigate why.

Screen Shot 2022-12-16 at 4 24 37 PM

Screen Shot 2022-12-16 at 4 24 46 PM

@suztomo suztomo added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p2 Moderately-important priority. Fix may not be included in next release. labels Dec 16, 2022
@lqiu96 lqiu96 self-assigned this Dec 21, 2022
@lqiu96
Copy link
Contributor

lqiu96 commented Dec 21, 2022

I was able to resolve the issue with the protobuf and grpc isues by clicking Unlinking Maven Project at the root pom.xml and then re-linking it.

Screen Shot 2022-12-21 at 4 55 33 PM

Unfortunately, this didn't resolve the testlib dependency issues in ServiceClientTestClassComposer. Taking another look for this class.

@lqiu96
Copy link
Contributor

lqiu96 commented Dec 21, 2022

This is my theory as of right now: SNAPSHOT dependencies are resolved via project source location

From this answer in https://stackoverflow.com/a/11545441

This seems to be the case as moving to a non -SNAPSHOT version has no issue pulling in these files. External Libraries show that the testlib is being pulled in (I only marked the testlibs as non--SNAPSHOT):
Screen Shot 2022-12-21 at 5 39 43 PM

I also marked the the test directory in each of gax's submodules (gax, gax-grpc, and gax-httpjson) as Sources Root also seems to resolve the ClassNotFound issue even when using a -SNAPSHOT version.

Screen Shot 2022-12-21 at 5 35 38 PM

TODO:
Looking to see if I can force Intellij to resolve -SNAPSHOT jars to pull from ~/.m2 local repository. Otherwise, I think we push an intellij settings file to the monorepo to treat gax's test dir as source dirs to fix the ClassNotFound exceptions in Intellij-only.

@suztomo
Copy link
Member Author

suztomo commented Dec 21, 2022

Thank you for looking into this issue.

I can force Intellij to resolve -SNAPSHOT jars to pull from ~/.m2 local repository.

Ideally, I want IntelliJ to pull the sources from the project, not from .m2 local repository (which requires "mvn install" every time we make changes to the sources).

an intellij settings file to the monorepo

If steps are simple, just adding how-to explanation in DEVELOPMENT.md suffices. (Maybe I'm not familiar with IntelliJ setting file, I worry that we might end up adding irrelevant settings, such as font color, to the file. )

@lqiu96
Copy link
Contributor

lqiu96 commented Dec 21, 2022

I can force Intellij to resolve -SNAPSHOT jars to pull from ~/.m2 local repository.

Ideally, I want IntelliJ to pull the sources from the project, not from .m2 local repository (which requires "mvn install" every time we make changes to the sources).

I see. Agreed, I think that makes sense and that running mvn install on every change would be a less than ideal development experience.

an intellij settings file to the monorepo

If steps are simple, just adding how-to explanation in DEVELOPMENT.md suffices. (Maybe I'm not familiar with IntelliJ setting file, I worry that we might end up adding irrelevant settings, such as font color, to the file. )

I'll do some digging and see how easy it is to add. Otherwise updating DEVELOPMENT.md should suffice.

@lqiu96
Copy link
Contributor

lqiu96 commented Dec 21, 2022

Actually, this seems to cause another issue. It's able to resolve the class not found from Intellij, but I can't click the Run all tests in the Gax repo from Intellij (now that it's recognized as Sources Root instead of Test Sources Root)...

I'll need to take another look...

@suztomo
Copy link
Member Author

suztomo commented Dec 22, 2022

Would you focus on investigating/writing down why this happens? (Rather than solutions yet)

I think the key is, I observe, "mvn verify" succeeds while "mvn test" fails. (The former does additional steps including packaging; ensure you remove ~/.m2/repository/com/google/api/gax* when experimenting)

@blakeli0
Copy link
Collaborator

blakeli0 commented Dec 22, 2022

Ideally, I want IntelliJ to pull the sources from the project, not from .m2 local repository (which requires "mvn install" every time we make changes to the sources).

I agree. Running mvn install on every change in other modules(gax-java and api-common-java) is also acceptable for now, because that's how it used to be and we are not making it worse.
@lqiu96 Can you please time box this to a day or two since your current assigned task is a cross language feature and has a target date to meet? As long as we have a working development workflow, I'm OK with it for now. @suztomo or anyone in the team can always come back to make it better.

@lqiu96
Copy link
Contributor

lqiu96 commented Dec 22, 2022

I'll have to revisit this in the future, but I'll try to add more details so anyone reading this thread can try and pick up where I left off:

Some classes not found can be resolved in #1144 (comment) via Linking and Unlinking Maven Project or Invalidating the IntelliJ caches. However, the testlib classes are still not found and I'll need to do more digging...


I wasn't aware that mvn test was failing as the I thought the maven build succeed and that this was only an IntelliJ only issue.

Interestingly, when you run mvn clean install/package/verify on the root of the monorepo, the project is able to pass. Run other lifecycle steps like mvn test and it fails to compile the gapic-generator-java module. But running mvn test or mvn compile inside the individual modules succeeds (i.e cd into gax-java or gapic-generator-java).

What I would expect to pass (but currently does not):

mvn clean install -> Pass and puts gax module and gax testlib module in ~/.m2
mvn test               -> Pass and compilation uses the testlib module in ~/.m2

I would think this would include put gax's testlib in the local ~/.m2 repository and the testlib be used by gapic-generator-java. This only seems to work when I force gapic-generator-java to pull in gax GAV coordinates that are different from what is in the gax module.


Temp workarounds

Note: This may or may not work and this may cause more issues

  1. Set the current Test Sources directory (src/test/java) as a Sources directory in IntelliJ so that the classes are able to be resolved -- may have issues building the project
  2. Manually specify the versions for gax deps to be the latest released version (released versions from maven-central) in gapic-generator-java pom.xml
  3. Leave as is and use the terminal to manually cd in the module you're working in and run the mvn commands there

@lqiu96 lqiu96 removed their assignment Dec 22, 2022
@suztomo
Copy link
Member Author

suztomo commented Jan 13, 2023

Thank you for troubleshooting.

Set the current Test Sources directory

When I mark "gax-java/gax-grpc/src/test/java" as Source Root, it resolves the classes. However, when I reload Maven project, the directory becomes "Test Source Root" again.

image

Manually specify the versions for gax deps to be the latest released version

This works nicely.

Screenshot 2023-01-13 at 10 56 53 AM

But of course we want to have it reference to the same GAX as in the repository.

I tried using it via a profile. But IntelliJ didn't recognize it when running a test.

image

@blakeli0
Copy link
Collaborator

blakeli0 commented Mar 9, 2023

@suztomo It's great to see you and @JoeWang1127 are working on improving the debugging experience of gapac-generator-java, is it possible to pick this up as well if you have some bandwidth? This has been a big pain to us that we can not run unit tests through intelliJ.

@suztomo
Copy link
Member Author

suztomo commented Mar 9, 2023 via email

@JoeWang1127
Copy link
Collaborator

JoeWang1127 commented Mar 9, 2023

I think #1474 can fix the issue. I got the idea from this post.

Though one more issue needs to consider: we need to change all modules in google-cloud-java from import gax:testlib, gax-grpc:testlib and gax-httpjson:testlib to gax:jar:testlib, gax-grpc:jar:testlib and gax-httpjson:jar:testlib

@JoeWang1127 JoeWang1127 linked a pull request Mar 9, 2023 that will close this issue
4 tasks
@suztomo
Copy link
Member Author

suztomo commented Mar 10, 2023

@JoeWang1127 Did you confirm your IntelliJ reproduces the problem?

@suztomo
Copy link
Member Author

suztomo commented Mar 10, 2023

Joe confirmed the problem.

@JoeWang1127
Copy link
Collaborator

JoeWang1127 commented Mar 10, 2023

I think the problem is we didn't include the test code when we package the gax module. In order to do that, we need <type>test-jar</type>.

Although in the post, the preferable way to fix the issue is to move the test classes to a dedicated module.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants