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

Normalize the quarkus.http.root-path at config level #19501

Merged
merged 1 commit into from
Aug 24, 2021

Conversation

gsmet
Copy link
Member

@gsmet gsmet commented Aug 19, 2021

Make sure the quarkus.http.root-path always starts and ends with a '/'
so that we don't have any risk of having logic working with '/test' and
not with '/test/'.

Fixes #19492

@gsmet
Copy link
Member Author

gsmet commented Aug 19, 2021

Converting to draft for now. I'll have a closer look at what's going on.

@gsmet gsmet marked this pull request as ready for review August 19, 2021 10:53
@gsmet gsmet changed the title Normalize the http-root-path at config level Normalize the quarkus.http.root-path at config level Aug 19, 2021
@gsmet
Copy link
Member Author

gsmet commented Aug 19, 2021

Should be ready now.

@gsmet gsmet marked this pull request as draft August 19, 2021 11:42
@gsmet
Copy link
Member Author

gsmet commented Aug 19, 2021

This is a bit of a can of worms. I'm making additional adjustments.

@gsmet gsmet marked this pull request as ready for review August 19, 2021 14:42
@quarkus-bot
Copy link

quarkus-bot bot commented Aug 19, 2021

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building 5062b6c

Status Name Step Test failures Logs Raw logs
JVM Tests - JDK 11 Build Test failures Logs Raw logs
JVM Tests - JDK 11 Windows Build Test failures Logs Raw logs
JVM Tests - JDK 16 Build Test failures Logs Raw logs

Full information is available in the Build summary check run.

Test Failures

⚙️ JVM Tests - JDK 11 #

📦 extensions/micrometer/deployment

io.quarkus.micrometer.deployment.binder.UriTagWithHttpApplicationRootTest.testRequestUris line 63 - More details - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <404>.

	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:490)
	at org.codehaus.groovy.reflection.CachedConstructor.invoke(CachedConstructor.java:72)
	at org.codehaus.groovy.reflection.CachedConstructor.doConstructorInvoke(CachedConstructor.java:59)
	at org.codehaus.groovy.runtime.callsite.ConstructorSite$ConstructorSiteNoUnwrap.callConstructor(ConstructorSite.java:84)
	at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCallConstructor(CallSiteArray.java:59)
	at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callConstructor(AbstractCa...

⚙️ JVM Tests - JDK 11 Windows #

📦 extensions/micrometer/deployment

io.quarkus.micrometer.deployment.binder.UriTagWithHttpApplicationRootTest.testRequestUris line 63 - More details - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <404>.

	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:490)
	at org.codehaus.groovy.reflection.CachedConstructor.invoke(CachedConstructor.java:72)
	at org.codehaus.groovy.reflection.CachedConstructor.doConstructorInvoke(CachedConstructor.java:59)
	at org.codehaus.groovy.runtime.callsite.ConstructorSite$ConstructorSiteNoUnwrap.callConstructor(ConstructorSite.java:84)
	at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCallConstructor(CallSiteArray.java:59)
	at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callConstructor(AbstractCa...

⚙️ JVM Tests - JDK 16 #

📦 extensions/micrometer/deployment

io.quarkus.micrometer.deployment.binder.UriTagWithHttpApplicationRootTest.testRequestUris line 63 - More details - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <404>.

	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:78)
	at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)
	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:480)
	at org.codehaus.groovy.reflection.CachedConstructor.invoke(CachedConstructor.java:72)
	at org.codehaus.groovy.runtime.callsite.ConstructorSite$ConstructorSiteNoUnwrapNoCoerce.callConstructor(ConstructorSite.java:105)
	at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCallConstructor(CallSiteArray.java:59)
	at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callConstructor(AbstractCallS...

@stuartwdouglas
Copy link
Member

LGTM in theory, but the failures look related.

@gsmet
Copy link
Member Author

gsmet commented Aug 19, 2021

Yeah... the servlet part is quite a mess on this side and the Micrometer tests are using servlet. I'm trying to clean this up and will push an update tomorrow morning once I have run more tests.

@gsmet
Copy link
Member Author

gsmet commented Aug 19, 2021

Removing the backport labels as it ends up being far more intrusive than I would have expected.

@quarkus-bot
Copy link

quarkus-bot bot commented Aug 20, 2021

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building 90aa803

Status Name Step Test failures Logs Raw logs
Gradle Tests - JDK 11 Windows Build Test failures Logs Raw logs
JVM Tests - JDK 11 Build Test failures Logs Raw logs
JVM Tests - JDK 11 Windows Build Test failures Logs Raw logs
JVM Tests - JDK 16 Build Test failures Logs Raw logs
Native Tests - Data7 Build Test failures Logs Raw logs

Full information is available in the Build summary check run.

Test Failures

⚙️ Gradle Tests - JDK 11 Windows #

📦 integration-tests/gradle

io.quarkus.gradle.devmode.InjectQuarkusAppPropertiesDevModeTest.main line 19 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Condition with lambda expression in io.quarkus.test.devmode.util.DevModeTestUtils that uses java.util.function.Supplier, java.util.function.Supplierjava.util.concurrent.atomic.AtomicReference, java.util.concurrent.atomic.AtomicReferencejava.lang.String, java.lang.Stringboolean was not fulfilled within 1 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:166)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:26)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:939)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:908)
	at io.quarkus.test.devmode.util.DevModeTestUtils.getHttpResponse(DevModeTestUtils.java:130)
	at io.quarkus.gradle.devmode.QuarkusDevGradleTestBase.getHttpResponse(QuarkusDevGradleTestBase.java:126)
	at io.quarkus.gradle.devmode.QuarkusDevGradleTestBase.getHttpRes...

⚙️ JVM Tests - JDK 11 #

📦 extensions/vertx-http/deployment

io.quarkus.vertx.http.deployment.HttpRootPathBuildItemTest.testResolvePathWithSlashApp line 21 - More details - Source on GitHub

org.opentest4j.AssertionFailedError: expected: </app/> but was: </app>
	at org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55)
	at org.junit.jupiter.api.AssertionUtils.failNotEqual(AssertionUtils.java:62)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:177)
	at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1124)
	at io.quarkus.vertx.http.deployment.HttpRootPathBuildItemTest.testResolvePathWithSlashApp(HttpRootPathBuildItemTest.java:21)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(Reflectio...

⚙️ JVM Tests - JDK 11 Windows #

📦 extensions/vertx-http/deployment

io.quarkus.vertx.http.deployment.HttpRootPathBuildItemTest.testResolvePathWithSlashApp line 21 - More details - Source on GitHub

org.opentest4j.AssertionFailedError: expected: </app/> but was: </app>
	at org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55)
	at org.junit.jupiter.api.AssertionUtils.failNotEqual(AssertionUtils.java:62)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:177)
	at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1124)
	at io.quarkus.vertx.http.deployment.HttpRootPathBuildItemTest.testResolvePathWithSlashApp(HttpRootPathBuildItemTest.java:21)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(Reflectio...

⚙️ JVM Tests - JDK 16 #

📦 extensions/vertx-http/deployment

io.quarkus.vertx.http.deployment.HttpRootPathBuildItemTest.testResolvePathWithSlashApp line 21 - More details - Source on GitHub

org.opentest4j.AssertionFailedError: expected: </app/> but was: </app>
	at org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55)
	at org.junit.jupiter.api.AssertionUtils.failNotEqual(AssertionUtils.java:62)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:177)
	at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1124)
	at io.quarkus.vertx.http.deployment.HttpRootPathBuildItemTest.testResolvePathWithSlashApp(HttpRootPathBuildItemTest.java:21)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:567)
	at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(Reflectio...

⚙️ Native Tests - Data7 #

📦 integration-tests/reactive-mysql-client

io.quarkus.it.reactive.mysql.client.NativeFruitsEndpointIT.testListAllFruits - More details - Source on GitHub

java.lang.RuntimeException: Unable to successfully launch process '43868'. Exit code is: '1'.
	at io.quarkus.test.common.LauncherUtil.ensureProcessIsAlive(LauncherUtil.java:96)
	at io.quarkus.test.common.LauncherUtil.waitForCapturedListeningData(LauncherUtil.java:65)
	at io.quarkus.test.common.DefaultNativeImageLauncher.start(DefaultNativeImageLauncher.java:116)
	at io.quarkus.test.junit.IntegrationTestUtil.startLauncher(IntegrationTestUtil.java:177)
	at io.quarkus.test.junit.NativeTestExtension.doNativeStart(NativeTestExtension.java:161)
	at io.quarkus.test.junit.NativeTestExtension.ensureStarted(NativeTestExtension.java:102)
	at io.quarkus.test.junit.NativeTestExtension.beforeAll(NativeTestExtension.java:75)
	at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.lambda$invokeBeforeAllCallbacks$8(ClassBasedTestDescriptor.java:368)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.jupiter.engine.descript...

@gsmet gsmet force-pushed the fix-19492 branch 2 times, most recently from 756eccc to c234c9a Compare August 20, 2021 08:33
Copy link
Member Author

@gsmet gsmet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ebullient I tweaked a few more things as the servlet path was not happy (and you use it in your Micrometer tests). I think I whacked all the moles but let's see what CI has to say.

I added a few comments at your intention. Could you have a look?

Also, I'll squash everything once settled.

@quarkus-bot
Copy link

quarkus-bot bot commented Aug 20, 2021

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building c234c9a

Status Name Step Test failures Logs Raw logs
Gradle Tests - JDK 11 Windows Build Test failures Logs Raw logs
✔️ JVM Tests - JDK 11
JVM Tests - JDK 11 Windows Build ⚠️ Check → Logs Raw logs
✔️ JVM Tests - JDK 16

Full information is available in the Build summary check run.

Test Failures

⚙️ Gradle Tests - JDK 11 Windows #

📦 integration-tests/gradle

io.quarkus.gradle.devmode.JandexMultiModuleProjectDevModeTest.main line 21 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Condition with lambda expression in io.quarkus.test.devmode.util.DevModeTestUtils that uses java.util.function.Supplier, java.util.function.Supplierjava.util.concurrent.atomic.AtomicReference, java.util.concurrent.atomic.AtomicReferencejava.lang.String, java.lang.Stringboolean was not fulfilled within 1 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:166)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:26)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:939)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:908)
	at io.quarkus.test.devmode.util.DevModeTestUtils.getHttpResponse(DevModeTestUtils.java:130)
	at io.quarkus.gradle.devmode.QuarkusDevGradleTestBase.getHttpResponse(QuarkusDevGradleTestBase.java:126)
	at io.quarkus.gradle.devmode.QuarkusDevGradleTestBase.getHttpRes...

@gsmet
Copy link
Member Author

gsmet commented Aug 23, 2021

@ebullient I included your comments and squashed the whole thing. It should be ready now. Figuring out the purpose of the build items and properly documenting them is a task for another day.

Copy link
Member

@ebullient ebullient left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made the same mistake twice in my suggestions. 🙄 sorry.

…config level

Make sure both paths always start and end with a '/'
so that we don't have any risk of having logic working with '/test' and
not with '/test/'.

Adjust the servlet context path too.

Also simplify a few things thanks to that.

Fixes quarkusio#19492
@gsmet
Copy link
Member Author

gsmet commented Aug 23, 2021

@ebullient fixed.

@quarkus-bot
Copy link

quarkus-bot bot commented Aug 23, 2021

Failing Jobs - Building 4a66f79

Status Name Step Test failures Logs Raw logs
✔️ JVM Tests - JDK 11
JVM Tests - JDK 16 Build Test failures Logs Raw logs

Full information is available in the Build summary check run.

Test Failures

⚙️ JVM Tests - JDK 16 #

📦 integration-tests/elytron-undertow

io.quarkus.it.undertow.elytron.BaseAuthRestTest.testPost line 28 - More details - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:78)
	at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)
	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:480)
	at org.codehaus.groovy.reflection.CachedConstructor.invoke(CachedConstructor.java:72)
	at org.codehaus.groovy.runtime.callsite.ConstructorSite$ConstructorSiteNoUnwrapNoCoerce.callConstructor(ConstructorSite.java:105)
	at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCallConstructor(CallSiteArray.java:59)
	at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callConstructor(AbstractCallS...

📦 integration-tests/hibernate-reactive-panache

io.quarkus.it.panache.reactive.PanacheFunctionalityTest.testPanacheFunctionality line 49 - More details - Source on GitHub

java.net.SocketTimeoutException: Read timed out
	at java.base/sun.nio.ch.NioSocketImpl.timedRead(NioSocketImpl.java:283)
	at java.base/sun.nio.ch.NioSocketImpl.implRead(NioSocketImpl.java:309)
	at java.base/sun.nio.ch.NioSocketImpl.read(NioSocketImpl.java:350)
	at java.base/sun.nio.ch.NioSocketImpl$1.read(NioSocketImpl.java:803)
	at java.base/java.net.Socket$SocketInputStream.read(Socket.java:976)
	at org.apache.http.impl.io.AbstractSessionInputBuffer.fillBuffer(AbstractSessionInputBuffer.java:161)
	at org.apache.http.impl.io.SocketInputBuffer.fillBuffer(SocketInputBuffer.java:82)
	at org.apache.http.impl.io.AbstractSessionInputBuffer.readLine(AbstractSessionInputBuffer.java:276)
	at org.apache.http.impl.conn.DefaultHttpResponseParser.parseHead(DefaultHttpResponseParser.java:138)
	at org.apache.http.impl.conn.DefaultHttpResponseParser.parseHead(DefaultHttpResponseParser.java:56)
	at org.apache.http.impl.io.AbstractMessageParser.parse(AbstractMessageParser.java:259)
	at org.apache.ht...

@gsmet gsmet merged commit ab3a4f8 into quarkusio:main Aug 24, 2021
@quarkus-bot quarkus-bot bot added this to the 2.3 - main milestone Aug 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Static Content with quarkus.http-root-context
3 participants