Skip to content

Conversation

@gian1200
Copy link
Contributor

@gian1200 gian1200 commented Feb 8, 2025

Closes #34935
Continuation of #35095

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.

Local tests (Windows):

Additional notes:

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 (hopefully it doesn't make other tests fail)

@quarkus-bot

This comment was marked as resolved.

@gian1200 gian1200 changed the title feat: make URL inherit TestHTTPEndpoint from class level Draft: feat: make URL inherit TestHTTPEndpoint from class level Feb 8, 2025
@gian1200 gian1200 marked this pull request as draft February 8, 2025 21:40
@gian1200 gian1200 changed the title Draft: feat: make URL inherit TestHTTPEndpoint from class level Draft: Make URL inherit TestHTTPEndpoint from class level Feb 8, 2025
@gian1200 gian1200 changed the title Draft: Make URL inherit TestHTTPEndpoint from class level Make URL inherit TestHTTPEndpoint from class level Feb 8, 2025
@gian1200 gian1200 force-pushed the feature/testhttpendpoint-inheritance branch from 8c4075e to f60ff81 Compare February 8, 2025 21:44
@geoand
Copy link
Contributor

geoand commented Feb 9, 2025

Seems reasonable!

@gian1200 gian1200 force-pushed the feature/testhttpendpoint-inheritance branch from f60ff81 to c75ead2 Compare February 16, 2025 02:22
@gian1200 gian1200 marked this pull request as ready for review February 16, 2025 02:47
@quarkus-bot
Copy link

quarkus-bot bot commented Feb 17, 2025

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit c75ead2.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

Thanks!

@geoand geoand merged commit 3555cbc into quarkusio:main Feb 17, 2025
55 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.21 - main milestone Feb 17, 2025
@quarkus-bot quarkus-bot bot added the kind/enhancement New feature or request label Feb 17, 2025
@geoand
Copy link
Contributor

geoand commented Feb 18, 2025

I am actually going to revert this because it can cause issues like #46336, meaning it can break existing tests and users have no way to recover.

Sorry I didn't anticipate this earlier

@gian1200
Copy link
Contributor Author

Sure, no problem. I didn't either 😔. A simple type validation should fix it (only inject URL fields). Will see if I can do it this weekend (unless someone is willing to do it earlier 🙃)

PS: during my testing I noticed that injection happens once for each test, not necessarily once per class (eg. 2 times for 2 tests in the same class). Is this expected or should I open an issue? It feels inefficient.

@geoand
Copy link
Contributor

geoand commented Feb 18, 2025

A simple type validation should fix it (only inject URL fields). Will see if I can do it this weekend (unless someone is willing to do it earlier 🙃)

I know, but that is unfortunately not enough. There could very well be tests in the wild that use URL (or String) as a field that is set by the test itself. Those cases would now break.

@gian1200
Copy link
Contributor Author

Made a new PR. Limited it's scope to only inject field when @TestHTTPResource is explicitly use (AS-IS functionality).
Also added additional tests to cover #46336 and similar.

PR: #46410

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

Labels

area/testing kind/enhancement New feature or request

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