Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork4.1k
Removeroutes-compiler-injected-routes-compilation-with-request-passed
scripted test as duplicate#13231
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
…ed` scripted test as duplicate
I was taking a quick look at this PR and also#13230. It's a bit much to analyse. |
ihostage commentedJun 9, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Yes, Matthias, you are absolutely right in common. I also was scared.
When I was working on this PR I was thinking about how to simplify our codebase. As you said a few corner cases were implemented only in one test and this made the cost of supporting these tests very expensive. Spending time on CI wasn't a priority for me, I wanted onlyone example (actually, two java/scala 😄) that covered all cases related to route compilation to simplify maintenance it in future.
But I already spent this time a made this analysis 😂 Found a few missed cases and added them in these PRs and removed all other cases as duplicated. |
I think
routes-compiler-injected-routes-compilation-with-request-passed
already covered byroutes-compiler-routes-compilation
androutes-compiler-routes-compilation-java
.I added a few cases related the name, type and position of
Request
param intoroutes-compiler-routes-compilation-java
. Therefore, I think we don't need this duplicated test now.