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

[BUG] Egeria Intellij build failure #7324

Closed
MihaiIliescu opened this issue Jan 26, 2023 · 15 comments
Closed

[BUG] Egeria Intellij build failure #7324

MihaiIliescu opened this issue Jan 26, 2023 · 15 comments
Assignees
Labels
bug Something isn't working triage New bug/issue which needs checking & assigning

Comments

@MihaiIliescu
Copy link
Contributor

Existing/related issue?

No response

Current Behavior

I pulled the up to date egeria repo from main and after the maven build I tried to run the build from the intellij interface and I got an error:

~/egeria/open-metadata-test/open-metadata-fvt/open-types-fvt/open-types-test/src/test/java/org/odpi/openmetadata/fvt/opentypes/server/TestBeansToAccessOMRS.java:126766:36
java: variable primitivePropertyValueForMap is already defined in method testCreateAgreementItem()

The class that fails is generated code and declares multiple times the same variable.

The issue is not only on my setup, I checked with a colleague and we got the error.

Expected Behavior

The build finishes successfully.

Steps To Reproduce

No response

Environment

- Egeria: 3.15-SNAPSHOT
- OS: MacOS Monterey v12.6.2 
- Java: 11
- Browser (for UI issues): -
- Additional connectors and integration: -
- Intellij Version my machine: IntelliJ IDEA 2022.3.1 (Ultimate Edition)
- Intellij Version colleague machine: IntelliJ IDEA 2021.1.3 (Ultimate Edition)

Any Further Information?

No response

@MihaiIliescu MihaiIliescu added bug Something isn't working triage New bug/issue which needs checking & assigning labels Jan 26, 2023
@davidradl
Copy link
Member

Locally I can build core Egeria and get a valid generated file TestBeansToAccessOMRS.java.
I can confirm that the IJ build creates an invalid version of TestBeansToAccessOMRS.java with duplicate declarations.

@planetf1
Copy link
Member

planetf1 commented Jan 26, 2023

I

  • cloned the egeria repo (with up to date main) - so completely clean
  • did a 'mvn clean install' at CLI
  • Opened the directory with intellij 2022.3.1 ie as a new project
  • Answered 'maven' when prompted for maven+gradle
  • Let IntelliJ index etc
  • Clicked 'build project'
  • Clicked ' enable annotation processing' when prompted

The resultant build was clean

I inspected the file, and at line 126766 of TestBeansToAccessOMRS.java I see

MapPropertyValue testMapPropertyValue = null;

which reports no errors - but also different content on the line to the error you are reporting.

I do have many declarations of primitivePropertyValueForMap in the file, and also in this method testCreateAgreementItem() it is declared 4 times

             PrimitivePropertyValue primitivePropertyValueForMap = new PrimitivePropertyValue();

But the end result is

26/01/2023, 12:20 - Build completed successfully with 645 warnings in 30 sec, 306 ms

I had left my JVM at default. On checking, I am using 'temurin 17' - and the language level defaulted to 17 (this is something I usually check/change after importing a project
I also checked in settings under Maven, and I am running the Bundled maven 3 (3.8.1)

The editor marks this file as 'you are editing a file which is ignored' - which is good, since intelliJ knows this file is generated

Additionally, there are no graphical overlays on main/java or main/test - ie IntelliJ is not seeing these as source modules (and so wouldn't try to compile, or report any issues), but arguably this is also true, since they are generated and just an intermediate step of the build process.

So it would seem there are maybe two issues

  • why do we have different behaviour in the IDE with some people seeing these files as buildable (I think it's right they are ignored? Maybe?)
  • why is the invalid file not used -- does this mean the code isn't even running? I would see the maven/gradle builds as definitive here?

@davidradl @MihaiIliescu is the sequence I followed exactly the same as you?

@planetf1
Copy link
Member

Under Help->Custom properties, I also set 'idea.max.intellisense.filesize=10000' which allowed IntelliJ to analyze even these large source files (mem/cpu usage higher) which clarifies these issues in the file
Screenshot 2023-01-26 at 12 52 29

@planetf1
Copy link
Member

For the gradle build, no code has been added to build/execute these tests at all - I missed this when doing the port
In the case of maven whilst test code is built, there is no execution. Looking at maven.

@davidradl
Copy link
Member

I think I have found a bug in the generator - just testing.

@davidradl
Copy link
Member

@planetf1 do you want to test #7325 .

@planetf1
Copy link
Member

I'm looking at why the tests don't run in our build. I can't see anything changed in maven for years .
However with our upcoming move to gradle I'll focus my fix there. Just experimenting at the moment - WITHOUT your fix, as first I went to see a compile failure! Then I will try together with your fix.

@davidradl
Copy link
Member

davidradl commented Jan 26, 2023

@planetf1 I will not merge until you give me the nod. @MihaiIliescu you could try applying the pr locally and test it works if you like

@davidradl davidradl self-assigned this Jan 26, 2023
@planetf1
Copy link
Member

I have fixed up gradle to actually build the tests.
Next I will incorporate the code fix, and verify the tests run.

@MihaiIliescu
Copy link
Contributor Author

yes, I'll test it now

@MihaiIliescu
Copy link
Contributor Author

Just finished testing #7325.

Pulled David's PR, ran mvn clean install, and then build from Intellij with no errors. Great work!

@planetf1
Copy link
Member

planetf1 commented Feb 3, 2023

There's a new PR #7327 which incorporates David's fix, but also corrects the gradle build to actually run the tests!

It was ready to merge, but blocked by many code style warnings from codeQL. Awaiting a decision as to what to do with those before we merge.

with the pending change to v4 I suggest this waits until after that (Monday 6 Feb)

@planetf1
Copy link
Member

planetf1 commented Mar 1, 2023

@mandy-chessell @dwolfson

@planetf1
Copy link
Member

planetf1 commented Mar 1, 2023

Though not directly related, I am intrigued if #7425 would make any difference

planetf1 added a commit to planetf1/egeria that referenced this issue Mar 1, 2023
Signed-off-by: Nigel Jones <nigel.l.jones+git@gmail.com>
planetf1 added a commit to planetf1/egeria that referenced this issue Mar 1, 2023
Signed-off-by: Nigel Jones <nigel.l.jones+git@gmail.com>
planetf1 added a commit to planetf1/egeria that referenced this issue Mar 7, 2023
Signed-off-by: Nigel Jones <nigel.l.jones+git@gmail.com>
planetf1 added a commit that referenced this issue Mar 7, 2023
#7324 Ensure Open Types tests are run (gradle)
@planetf1
Copy link
Member

This should be fixed now, though note issue #7536

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triage New bug/issue which needs checking & assigning
Projects
None yet
Development

No branches or pull requests

3 participants