Allow multiple source directories for Gradle. #5540

soberich · 2019-11-17T11:25:18Z

Kotlin, annotation processors and/or multi-module projects required multiple directories as sourceDir.

Fixes #5539

geoand · 2019-11-18T07:35:32Z

Hi,

Can you please rebase onto master to pick up a fix for CI?

Thanks

soberich · 2019-11-18T09:48:59Z

@geoand done. Let's see the CI.

soberich · 2019-11-18T13:18:00Z

@geoand Looks like some ordering problem in Microprofile TCK tests. Not from Quarkus side.
These lines does respect the order, while they should probably not:

https://github.com/eclipse/microprofile-opentracing/blob/0d66079f00f15f6c3be8b5c33b5a0dd60a2baa07/tck/base/src/main/java/org/eclipse/microprofile/opentracing/tck/tracer/TestSpanTree.java#L206-L212

geoand · 2019-11-18T13:33:06Z

For future reference, we are seeing:

[ERROR] Tests run: 5, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 10.963 s <<< FAILURE! - in org.eclipse.microprofile.opentracing.tck.rest.client.OpenTracingMpRestClientTests
[ERROR] testMethodNotTraced(org.eclipse.microprofile.opentracing.tck.rest.client.OpenTracingMpRestClientTests)  Time elapsed: 0.058 s  <<< FAILURE!
java.lang.AssertionError: 
expected [[
  {
    span: "{ operationName: GET:org.eclipse.microprofile.opentracing.tck.application.TestServerWebServices.simpleTest, tags: [component=jaxrs, http.method=GET, http.status_code=200, http.url=http://localhost:8081/rest/testServices/simpleTest, span.kind=server], logEntries: []}"
  },
  {
    span: "{ operationName: GET:org.eclipse.microprofile.opentracing.tck.rest.client.RestClientServices.restClientMethodTracingDisabled, tags: [component=jaxrs, http.method=GET, http.status_code=200, http.url=http://localhost:8081/rest/mpRestClient/restClientMethodTracingDisabled, span.kind=server], logEntries: []}"
  }
]
] but found [[
  {
    span: "{ operationName: GET:org.eclipse.microprofile.opentracing.tck.rest.client.RestClientServices.restClientMethodTracingDisabled, tags: [component=jaxrs, http.method=GET, http.status_code=200, http.url=http://localhost:8081/rest/mpRestClient/restClientMethodTracingDisabled, span.kind=server], logEntries: []}"
  },
  {
    span: "{ operationName: GET:org.eclipse.microprofile.opentracing.tck.application.TestServerWebServices.simpleTest, tags: [component=jaxrs, http.method=GET, http.status_code=200, http.url=http://localhost:8081/rest/testServices/simpleTest, span.kind=server], logEntries: []}"
  }
]
]
	at org.testng.Assert.fail(Assert.java:96)
	at org.testng.Assert.failNotEquals(Assert.java:776)
	at org.testng.Assert.assertEqualsImpl(Assert.java:137)
	at org.testng.Assert.assertEquals(Assert.java:118)
	at org.testng.Assert.assertEquals(Assert.java:442)
	at org.eclipse.microprofile.opentracing.tck.OpenTracingBaseTests.assertEqualTrees(OpenTracingBaseTests.java:227)
	at org.eclipse.microprofile.opentracing.tck.rest.client.OpenTracingMpRestClientTests.testNotTraced(OpenTracingMpRestClientTests.java:166)
	at org.eclipse.microprofile.opentracing.tck.rest.client.OpenTracingMpRestClientTests.testMethodNotTraced(OpenTracingMpRestClientTests.java:116)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:124)
	at org.testng.internal.MethodInvocationHelper$1.runTestMethod(MethodInvocationHelper.java:230)
	at org.jboss.arquillian.testng.Arquillian$2.invoke(Arquillian.java:145)
	at org.jboss.arquillian.container.test.impl.execution.LocalTestExecuter.execute(LocalTestExecuter.java:57)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.jboss.arquillian.core.impl.ObserverImpl.invoke(ObserverImpl.java:86)
	at org.jboss.arquillian.core.impl.EventContextImpl.invokeObservers(EventContextImpl.java:103)
	at org.jboss.arquillian.core.impl.EventContextImpl.proceed(EventContextImpl.java:90)
	at org.jboss.arquillian.core.impl.ManagerImpl.fire(ManagerImpl.java:133)
	at org.jboss.arquillian.core.impl.ManagerImpl.fire(ManagerImpl.java:105)
	at org.jboss.arquillian.core.impl.EventImpl.fire(EventImpl.java:62)
	at org.jboss.arquillian.container.test.impl.execution.ClientTestExecuter.execute(ClientTestExecuter.java:50)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.jboss.arquillian.core.impl.ObserverImpl.invoke(ObserverImpl.java:86)
	at org.jboss.arquillian.core.impl.EventContextImpl.invokeObservers(EventContextImpl.java:103)
	at org.jboss.arquillian.core.impl.EventContextImpl.proceed(EventContextImpl.java:90)
	at org.jboss.arquillian.container.test.impl.client.ContainerEventController.createContext(ContainerEventController.java:128)
	at org.jboss.arquillian.container.test.impl.client.ContainerEventController.createTestContext(ContainerEventController.java:118)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.jboss.arquillian.core.impl.ObserverImpl.invoke(ObserverImpl.java:86)
	at org.jboss.arquillian.core.impl.EventContextImpl.proceed(EventContextImpl.java:95)
	at org.jboss.arquillian.test.impl.TestContextHandler.createClassContext(TestContextHandler.java:83)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.jboss.arquillian.core.impl.ObserverImpl.invoke(ObserverImpl.java:86)
	at org.jboss.arquillian.core.impl.EventContextImpl.proceed(EventContextImpl.java:95)
	at org.jboss.arquillian.test.impl.TestContextHandler.createSuiteContext(TestContextHandler.java:69)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.jboss.arquillian.core.impl.ObserverImpl.invoke(ObserverImpl.java:86)
	at org.jboss.arquillian.core.impl.EventContextImpl.proceed(EventContextImpl.java:95)
	at org.jboss.arquillian.test.impl.TestContextHandler.createTestContext(TestContextHandler.java:116)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.jboss.arquillian.core.impl.ObserverImpl.invoke(ObserverImpl.java:86)
	at org.jboss.arquillian.core.impl.EventContextImpl.proceed(EventContextImpl.java:95)
	at org.jboss.arquillian.core.impl.ManagerImpl.fire(ManagerImpl.java:133)
	at org.jboss.arquillian.test.impl.EventTestRunnerAdaptor.test(EventTestRunnerAdaptor.java:139)
	at org.jboss.arquillian.testng.Arquillian.run(Arquillian.java:138)
	at org.testng.internal.MethodInvocationHelper.invokeHookable(MethodInvocationHelper.java:242)
	at org.testng.internal.Invoker.invokeMethod(Invoker.java:576)
	at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:716)
	at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:988)
	at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:125)
	at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:109)
	at org.testng.TestRunner.privateRun(TestRunner.java:648)
	at org.testng.TestRunner.run(TestRunner.java:505)
	at org.testng.SuiteRunner.runTest(SuiteRunner.java:455)
	at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:450)
	at org.testng.SuiteRunner.privateRun(SuiteRunner.java:415)
	at org.testng.SuiteRunner.run(SuiteRunner.java:364)
	at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
	at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:84)
	at org.testng.TestNG.runSuitesSequentially(TestNG.java:1208)
	at org.testng.TestNG.runSuitesLocally(TestNG.java:1137)
	at org.testng.TestNG.runSuites(TestNG.java:1049)
	at org.testng.TestNG.run(TestNG.java:1017)
	at org.apache.maven.surefire.testng.TestNGExecutor.run(TestNGExecutor.java:135)
	at org.apache.maven.surefire.testng.TestNGDirectoryTestSuite.executeSingleClass(TestNGDirectoryTestSuite.java:112)
	at org.apache.maven.surefire.testng.TestNGDirectoryTestSuite.execute(TestNGDirectoryTestSuite.java:99)
	at org.apache.maven.surefire.testng.TestNGProvider.invoke(TestNGProvider.java:146)
	at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:384)
	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:345)
	at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:126)
	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:418)

[INFO] 
[INFO] Results:
[INFO] 
[ERROR] Failures: 
[ERROR]   OpenTracingMpRestClientTests>Arquillian.run:138->testMethodNotTraced:116->testNotTraced:166->OpenTracingBaseTests.assertEqualTrees:227 expected [[
  {
    span: "{ operationName: GET:org.eclipse.microprofile.opentracing.tck.application.TestServerWebServices.simpleTest, tags: [component=jaxrs, http.method=GET, http.status_code=200, http.url=http://localhost:8081/rest/testServices/simpleTest, span.kind=server], logEntries: []}"
  },
  {
    span: "{ operationName: GET:org.eclipse.microprofile.opentracing.tck.rest.client.RestClientServices.restClientMethodTracingDisabled, tags: [component=jaxrs, http.method=GET, http.status_code=200, http.url=http://localhost:8081/rest/mpRestClient/restClientMethodTracingDisabled, span.kind=server], logEntries: []}"
  }
]
] but found [[
  {
    span: "{ operationName: GET:org.eclipse.microprofile.opentracing.tck.rest.client.RestClientServices.restClientMethodTracingDisabled, tags: [component=jaxrs, http.method=GET, http.status_code=200, http.url=http://localhost:8081/rest/mpRestClient/restClientMethodTracingDisabled, span.kind=server], logEntries: []}"
  },
  {
    span: "{ operationName: GET:org.eclipse.microprofile.opentracing.tck.application.TestServerWebServices.simpleTest, tags: [component=jaxrs, http.method=GET, http.status_code=200, http.url=http://localhost:8081/rest/testServices/simpleTest, span.kind=server], logEntries: []}"
  }
]
]
[INFO]

@kenfinnigan @objectiser any idea?

kenfinnigan · 2019-11-18T13:47:05Z

I think the order is important, as it implies the spans were collected in a different sequence

geoand · 2019-11-18T15:40:09Z

I restarted the failing test, let's see what happens

geoand · 2019-11-18T17:27:01Z

Test failure seems to be unrelated (##5569)

soberich · 2019-11-20T07:31:21Z

@geoand I reran. It's fine now.

geoand · 2019-11-20T07:32:08Z

@aloubyansky @gastaldi mind taking a look please?

soberich · 2019-11-20T07:33:28Z

@geoand BTW. After say it is merged, is this going to be in snapshot repo as 999-SNAPSHOT?

aloubyansky

Looks good to me, I haven't tried it though. It'd be good to have a test for this. Thanks a lot @soberich
I'll let @gastaldi to comment before merging this.

geoand · 2019-11-20T08:02:07Z

@soberich everything build locally uses 999-SNAPSHOT. Is that what you mean?

gastaldi

LGTM, as @aloubyansky mentioned, it would be nice to have some tests too

gsmet · 2019-11-20T19:15:44Z

@gastaldi I thought we didn't have any Gradle ITs anyway? Or did the situation change?

gastaldi · 2019-11-20T19:22:52Z

@gsmet you're right, we need to add some and change the pom.xml to build using Gradle instead

gsmet · 2019-11-20T19:27:01Z

So, given we don't have any tests, did someone try this manually and confirm it's working as advertised.

If so let's merge. The bad Gradle testing situation shouldn't block this PR.

geoand · 2019-11-22T19:07:42Z

I'll hopefully try this over the weekend

cescoffier · 2019-11-23T07:50:48Z

@geoand I let you merge once you checked it.

…or multi-module projects required multiple directories as sourceDir.

soberich · 2019-11-23T17:01:46Z

It doesn't work. The approach taken by Quarkus plugin hardly fits in Gradle integration. In many cases Gradle will have multiple dirs in sources, classes, resources, etc in project layout.
Quarkus plugin is too opinionated on in this sence. It expects something like absolute path for a single file or java.nio.file.Path in many places which is not the case.
For example io.quarkus.dev.DevModeContext.ModuleInfo#ModuleInfo ctor expects String classesPath,... while in reality it will be

build
    classes
        java
        kotlin
        scala
    ...

aslo io.quarkus.creator.phase.augment.AugmentTask.Builder#appClassesDir
also java.lang.ProcessBuilder#directory(java.io.File) calls should simply get /build or /build/classes, because now it gets path:path:path from Gradle.
so without re-write of quarkus-development-mode it will be a crutch.
Ideally all api for development mode should assume Collection of Files.

geoand · 2019-11-24T10:42:30Z

I tested it with a generated and it works great. Thanks a lot @soberich!

As for your comment above about the re-write, I'll ping @aloubyansky and @maxandersen so they can have that in mind for the future plans

soberich · 2019-11-24T11:03:32Z

@geoand It does cover a bunch of cases but definitely not all of them. I want to open another PR because locally I already started something. I am not sure how fast would it go. Is this ok?

soberich · 2019-11-24T11:09:52Z

Actually, I am not sure we should've merge it.
@geoand I thought after my comment you won't. Because there is something quite important.
For example, I got NPE here

quarkus/devtools/gradle/src/main/java/io/quarkus/gradle/tasks/QuarkusBuild.java

Line 82 in 2ea45ab

if (uberJar && System.getProperty("quarkus.package.uber-jar") == null) {

uberJar is null and unboxing throws.

geoand · 2019-11-24T11:13:06Z

@soberich OK, I opened a PR to revert this one: #5727.
I'll merge it and then you can work on a new one as you see fit. Does that sound good?

aloubyansky approved these changes Nov 20, 2019

View reviewed changes

gastaldi approved these changes Nov 20, 2019

View reviewed changes

soberich added 2 commits November 23, 2019 15:00

Allow multiple source directories. Kotlin, annotation processors and/…

3640f22

…or multi-module projects required multiple directories as sourceDir.

Quote on splitting for Windows.

f61393d

Gradle 6.x compatibility.

b788af7

geoand added this to the 1.1.0 milestone Nov 24, 2019

geoand merged commit 14bd117 into quarkusio:master Nov 24, 2019

geoand mentioned this pull request Nov 24, 2019

Revert "Allow multiple source directories for Gradle." #5727

Merged

gsmet added the release/noteworthy-feature label Nov 24, 2019

gsmet added triage/invalid This doesn't seem right and removed release/noteworthy-feature labels Nov 24, 2019

gsmet removed this from the 1.1.0 milestone Nov 24, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow multiple source directories for Gradle. #5540

Allow multiple source directories for Gradle. #5540

soberich commented Nov 17, 2019

geoand commented Nov 18, 2019 •

edited

Loading

soberich commented Nov 18, 2019

soberich commented Nov 18, 2019

geoand commented Nov 18, 2019

kenfinnigan commented Nov 18, 2019

geoand commented Nov 18, 2019

geoand commented Nov 18, 2019

soberich commented Nov 20, 2019

geoand commented Nov 20, 2019

soberich commented Nov 20, 2019

aloubyansky left a comment

geoand commented Nov 20, 2019

gastaldi left a comment

gsmet commented Nov 20, 2019

gastaldi commented Nov 20, 2019

gsmet commented Nov 20, 2019

geoand commented Nov 22, 2019

cescoffier commented Nov 23, 2019

soberich commented Nov 23, 2019 •

edited

Loading

geoand commented Nov 24, 2019 •

edited

Loading

soberich commented Nov 24, 2019

soberich commented Nov 24, 2019

geoand commented Nov 24, 2019

Allow multiple source directories for Gradle. #5540

Allow multiple source directories for Gradle. #5540

Conversation

soberich commented Nov 17, 2019

geoand commented Nov 18, 2019 • edited Loading

soberich commented Nov 18, 2019

soberich commented Nov 18, 2019

geoand commented Nov 18, 2019

kenfinnigan commented Nov 18, 2019

geoand commented Nov 18, 2019

geoand commented Nov 18, 2019

soberich commented Nov 20, 2019

geoand commented Nov 20, 2019

soberich commented Nov 20, 2019

aloubyansky left a comment

Choose a reason for hiding this comment

geoand commented Nov 20, 2019

gastaldi left a comment

Choose a reason for hiding this comment

gsmet commented Nov 20, 2019

gastaldi commented Nov 20, 2019

gsmet commented Nov 20, 2019

geoand commented Nov 22, 2019

cescoffier commented Nov 23, 2019

soberich commented Nov 23, 2019 • edited Loading

geoand commented Nov 24, 2019 • edited Loading

soberich commented Nov 24, 2019

soberich commented Nov 24, 2019

geoand commented Nov 24, 2019

geoand commented Nov 18, 2019 •

edited

Loading

soberich commented Nov 23, 2019 •

edited

Loading

geoand commented Nov 24, 2019 •

edited

Loading