search

getindata / flink-http-connector

Http Connector for Apache Flink. Provides sources and sinks for Datastream , Table and SQL APIs.
Apache License 2.0
136 stars 39 forks source link

issue 64 add junit 5 support #72

Closed davidradl closed 3 months ago

davidradl commented 4 months ago

Description

Add junit 5 support - so tests can run against Flink 1.17 and 1.18 Resolves #64

PR Checklist
kristoffSC commented 3 months ago

checking

kristoffSC commented 3 months ago

checking.

kristoffSC commented 3 months ago

Hi @davidradl Please try this patch on your branch and let me know what are the results

Flink1.16.patch

kristoffSC commented 3 months ago

I see thatbuild fails with below exception

Error: 9,649 [ERROR]   HttpSinkConnectionTest.testFailedConnection:238 expected: <1> but was: <0>
Error: 9,649 [ERROR]   HttpSinkConnectionTest.testServerErrorConnection:202 expected: <1> but was: <0>

It is an improvement comparing to previous thing where thre was classNoFound ;) I will take a look at those two tests, maybe I will find what causing the issue here.

UPDATE: This is working on Flink 1.16.3 interesting i bet we are doing something wrong in test for metric gathering.

davidradl commented 3 months ago

I see thatbuild fails with below exception

Error: 9,649 [ERROR]   HttpSinkConnectionTest.testFailedConnection:238 expected: <1> but was: <0>
Error: 9,649 [ERROR]   HttpSinkConnectionTest.testServerErrorConnection:202 expected: <1> but was: <0>

It is an improvement comparing to previous thing where thre was classNoFound ;) I will take a look at those two tests, maybe I will find what causing the issue here.

UPDATE: This is working on Flink 1.16.3 interesting i bet we are doing something wrong in test for metric gathering.

@kristoffSC thanks let me know how you get on.

davidradl commented 3 months ago

this pr may address https://github.com/getindata/flink-http-connector/issues/73

kristoffSC commented 3 months ago

Hi @davidradl Please apply attached patch on top of what you have currently on this branch. This should do the job.

I've ran local builds against Flink 1.16, 17 and 18 they all passing.

Flink1.16_b.patch

davidradl commented 3 months ago

@kristoffSC It is working - thanks for the patches - I have squashed the commits. I think we are ready to merge.