Convert netty 4.0 test to java#9976
Conversation
| @CsvSource({"by instance", "by class", "by name", "first"}) | ||
| void testRemoveOurHandler(String testName) throws Exception { | ||
| EmbeddedChannel channel = new EmbeddedChannel(new NoopChannelHandler()); | ||
| ChannelPipeline channelPipeline = channel.pipeline(); |
There was a problem hiding this comment.
DefaultChannelPipeline' is package protected in io.netty:netty-transport-4.0.0.Final. calling channel.pipeline()' will automatically add two default HeadHandler and TailHandler
that's why channelPipeline.first() and last() are not null anymore.
There was a problem hiding this comment.
you could get around this by creating the instance with reflection or by placing a helper class in the same package as DefaultChannelPipeline and call the constructor using that class
There was a problem hiding this comment.
@laurit i tried Class.forName("io.netty.channel.DefaultChannelPipeline"), but it returned the wrong class though.
There are a few of this class with the same package name..
such as org.asynchttpclient:netty-resolver-dns:2.0.0-RC9
or from different netty-transport ..
is there a way i can force it to retrieve it directly from netty-transport-4.0.0.FInal, which is used in the build.gradle.kts. but when i ran the test in intellij, it always retrieved from the wrong dependencies./
It's just an issue with IntelliJ.
| // regression test for | ||
| // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/4056 | ||
| @Test | ||
| void shouldAddAfterAndRemoveLastHandler() { |
There was a problem hiding this comment.
i modified this test based on the current implementation. please let me know if this is not the case, might be regression?
i might need to update netty 4.0 instrumentation for this. but i need some feedback first.
| assertEquals(httpHandler, channelPipeline.first()); | ||
| assertEquals(noopHandler, channelPipeline.last()); | ||
|
|
||
| // removeLast will remove everything after http |
There was a problem hiding this comment.
removeLast will remove everything after http including noopHandler.
There was a problem hiding this comment.
this is the same though using reflection
|
need some help here:
Netty40ServerTest is green locally, but CI complained about KtorHttpServerTest: KtorHttpServerTest > requestWithException() STANDARD_OUT is this sporadic? |
|
rerun failed jobs button is nowhere to be found. do i need additional permission to get access to it? |
I believe so, you can push an empty commit to trigger checks. Closing and reopening the pr should also work. |
|
@heyams I suspect the test failures might not be the ktor tests you referenced. Your screenshot shows passing tests when running the 
There are also different variations of the tests 
When trying to run some of those ( 
|
| // http and instrumentation handlers will be remained; noop handler will be removed | ||
| // but in reality, line 162 will remove http, instrumentation, and noop handlers |
There was a problem hiding this comment.
please read my comments here.
There was a problem hiding this comment.
The issue is in something else. Also note that using line numbers in the comment that way isn't the best idea as the line numbers can easily change.
part of #7195