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

conver servlet 5 tests from groovy to java #12364

Merged
merged 17 commits into from
Oct 19, 2024

Conversation

zeitlinger
Copy link
Member

@zeitlinger zeitlinger commented Sep 30, 2024

Created by copying the servlet 3 tests and then applying the diffs from between the groovy servlet 3 and groovy servlet 5 tests

@zeitlinger zeitlinger self-assigned this Sep 30, 2024
@zeitlinger zeitlinger requested a review from a team as a code owner September 30, 2024 12:15
@zeitlinger zeitlinger changed the title conver servlet 5 tests to java conver servlet 5 tests from groovy to java Sep 30, 2024
@zeitlinger
Copy link
Member Author

@laurit do I need to create a jetty11 testing project to get rid of this?

> Task :instrumentation:servlet:servlet-5.0:testing:compileTestJava
/home/runner/.gradle/caches/modules-2/files-2.1/org.eclipse.jetty/jetty-server/11.0.0/2ba1ab6b6ddacc67140a250c7ffb1770f23db901/jetty-server-11.0.0.jar(/org/eclipse/jetty/server/Request.class): warning: Cannot find annotation method 'since()' in type 'Deprecated'
/home/runner/.gradle/caches/modules-2/files-2.1/org.eclipse.jetty/jetty-server/11.0.0/2ba1ab6b6ddacc67140a250c7ffb1770f23db901/jetty-server-11.0.0.jar(/org/eclipse/jetty/server/Request.class): warning: Cannot find annotation method 'since()' in type 'Deprecated'
error: warnings found and -Werror specified

@laurit
Copy link
Contributor

laurit commented Oct 1, 2024

We have two kinds of modules that are named testing. Firstly there are module that contain classes that are shared by multiple modules that contain tests, e.g. a common testing module for javaagent and library modules. Secondly we have modules that contain the tests because for whatever reason they can't be in the same module as the instrumentation. I don't think we ever mix the two purposes. You could also do the same here by keeping the testing module as shared code and moving the tomcat tests together with instrumentation or to a separate module.

@zeitlinger
Copy link
Member Author

@laurit can you check again?

2 similar comments
@zeitlinger
Copy link
Member Author

@laurit can you check again?

@zeitlinger
Copy link
Member Author

@laurit can you check again?

@trask trask merged commit c165ca4 into open-telemetry:main Oct 19, 2024
56 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants