Skip to content

Conversation

@gian1200
Copy link
Contributor

I was unable to compile the project with VSCode (the contributing page doesn't talk much about it), but ./mvnw "-Dquickly" passed successfully.

  • Functionality:
    If a Test Class is annotated with @TestHTTPEndpoint then all URL fields inherit it's configuration.
    If URL is also annotated, URL's annotation takes precedence.

  • Tests:
    I couldn't find the tests for original functionality, so I included both (new and original).
    I reused some resources/endpoints. Not sure if that's acceptable/expected or not.

Closes #34935

@quarkus-bot

This comment has been minimized.

@geoand
Copy link
Contributor

geoand commented Jul 31, 2023

The test failures are definitely related

@gian1200
Copy link
Contributor Author

That is correct. There was an scenario that I was considering in my tests but was not already supported in the code.

I also managed to run the tests with ./mvnw test -f integration-tests/main/ -Dtest=TestHTTPResourceFieldTestCase and ./mvnw test -f integration-tests/main/ -Dtest=TestHTTPResourceClassTestCase so it shouldn't fail now (now I know that ./mvnw "-Dquickly" doesn't run these tests).

I'm only not sure about my last change, specially TestHTTPResourceManager.java:102 (I implemented it this way because ssl's default value of @TestHTTPResource is false).
The code was originally executed only when @TestHTTPResource was used. In this new code it is no longer necessary when using @TestHTTPEndpoint at class or field level. When this happens, I default the path to "" (which is the default value for @TestHTTPResource).

@geoand
Copy link
Contributor

geoand commented Aug 1, 2023

Can you squash the commits please?

@gian1200 gian1200 force-pushed the main branch 2 times, most recently from b614e33 to b61cee6 Compare August 1, 2023 13:44
@gian1200
Copy link
Contributor Author

gian1200 commented Aug 1, 2023

Done

}
String val;
if (resource.ssl()) {
if (resource != null && resource.ssl()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

How come this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's the one I wasn't sure about. If there is @TestHTTPEndpoint, but no @TestHTTPResource, then resource is null.

I kept it like that because @TestHTTPResource's SSL's default value is false.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, let's see what see CI says

@quarkus-bot
Copy link

quarkus-bot bot commented Aug 1, 2023

Failing Jobs - Building b61cee6

Status Name Step Failures Logs Raw logs
JVM Tests - JDK 11 Build Failures Logs Raw logs
JVM Tests - JDK 17 Build Failures Logs Raw logs
JVM Tests - JDK 17 Windows Build Failures Logs Raw logs
JVM Tests - JDK 20 Build Failures Logs Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 11 #

- Failing: integration-tests/opentelemetry 

📦 integration-tests/opentelemetry

io.quarkus.it.opentelemetry.EndUserDisabledTest.baseTest - More details - Source on GitHub

org.junit.jupiter.api.extension.TestInstantiationException: Failed to create test instance
	at io.quarkus.test.junit.QuarkusTestExtension.initTestState(QuarkusTestExtension.java:786)
	at io.quarkus.test.junit.QuarkusTestExtension.interceptTestClassConstructor(QuarkusTestExtension.java:752)

io.quarkus.it.opentelemetry.EndUserEnabledTest.baseTest - More details - Source on GitHub

org.junit.jupiter.api.extension.TestInstantiationException: Failed to create test instance
	at io.quarkus.test.junit.QuarkusTestExtension.initTestState(QuarkusTestExtension.java:786)
	at io.quarkus.test.junit.QuarkusTestExtension.interceptTestClassConstructor(QuarkusTestExtension.java:752)

⚙️ JVM Tests - JDK 17 #

- Failing: integration-tests/opentelemetry 

📦 integration-tests/opentelemetry

io.quarkus.it.opentelemetry.EndUserDisabledTest.baseTest - More details - Source on GitHub

org.junit.jupiter.api.extension.TestInstantiationException: Failed to create test instance
	at io.quarkus.test.junit.QuarkusTestExtension.initTestState(QuarkusTestExtension.java:786)
	at io.quarkus.test.junit.QuarkusTestExtension.interceptTestClassConstructor(QuarkusTestExtension.java:752)

io.quarkus.it.opentelemetry.EndUserEnabledTest.baseTest - More details - Source on GitHub

org.junit.jupiter.api.extension.TestInstantiationException: Failed to create test instance
	at io.quarkus.test.junit.QuarkusTestExtension.initTestState(QuarkusTestExtension.java:786)
	at io.quarkus.test.junit.QuarkusTestExtension.interceptTestClassConstructor(QuarkusTestExtension.java:752)

⚙️ JVM Tests - JDK 17 Windows #

- Failing: extensions/vertx-http/deployment 
! Skipped: extensions/agroal/deployment extensions/avro/deployment extensions/cache/deployment and 333 more

📦 extensions/vertx-http/deployment

Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:3.1.2:test (default-test) on project quarkus-vertx-http-deployment: There was a timeout in the fork


⚙️ JVM Tests - JDK 20 #

- Failing: integration-tests/opentelemetry 

📦 integration-tests/opentelemetry

io.quarkus.it.opentelemetry.EndUserDisabledTest.baseTest - More details - Source on GitHub

org.junit.jupiter.api.extension.TestInstantiationException: Failed to create test instance
	at io.quarkus.test.junit.QuarkusTestExtension.initTestState(QuarkusTestExtension.java:786)
	at io.quarkus.test.junit.QuarkusTestExtension.interceptTestClassConstructor(QuarkusTestExtension.java:752)

io.quarkus.it.opentelemetry.EndUserEnabledTest.baseTest - More details - Source on GitHub

org.junit.jupiter.api.extension.TestInstantiationException: Failed to create test instance
	at io.quarkus.test.junit.QuarkusTestExtension.initTestState(QuarkusTestExtension.java:786)
	at io.quarkus.test.junit.QuarkusTestExtension.interceptTestClassConstructor(QuarkusTestExtension.java:752)

@gian1200
Copy link
Contributor Author

gian1200 commented Aug 2, 2023

Not sure what the timeout error might be about. Should I rebase it again?

@geoand
Copy link
Contributor

geoand commented Aug 2, 2023

The test failures do seem to be related to the change - see the report

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/testing triage/invalid This doesn't seem right

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make TestHTTPEndpoint at class level affect all URL fields in test classes

2 participants