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

[MRESOLVER-449] Step toward testing all HTTP transports #391

Merged
merged 16 commits into from Dec 11, 2023

Conversation

cstamas
Copy link
Member

@cstamas cstamas commented Dec 7, 2023

Right now, the HTTP test suite is pulled out and reused for all HTTP transports. It passes for ApacheTransporter (as it was pulled from it), but Jetty and JDK show some signs of some problems...

Current results (note: base class HttpTransporterTest has 70 tests, while some transport specific UTs add more):

  • apache ✔️ 74 test OK
  • jdk 🔴 62 test OK, 9 test FAIL
  • jetty 🔴 64 test OK, 6 FAIL

The "basics" are overall good (basics pass everywhere), failures stems from small differences.

Jetty: it seems it consumes the body (pulls bytes out of it) even before all the HTTP auth happens, this causes that TransportListener gets notified "transport started" but in fact is not and test asserts it to not. Also, Jetty throws in tests where server is set up to abruptly close the connection. Retries are not properly done (low level, like abrupt connection drop).

JDK: same problem with body (pulls bytes beforehand) and similarly as with Jetty, "transport started" events fails the test. Preemptive auth is not done (as it seems it is either "manual" handling of auth or Authenticator is used but then preemptive auth becomes impossible as headers are removed, no way to set them). It also throws in tests where server abruptly drops connection. Same for retries (in case of TCP issue).


https://issues.apache.org/jira/browse/MRESOLVER-449

@cstamas cstamas changed the title Step toward testing all HTTP transports [MRESOLVER-449] Step toward testing all HTTP transports Dec 7, 2023
@cstamas cstamas added this to the 2.0.0 milestone Dec 8, 2023
And addition of Jetty request content that adapts PutTask
This will need some proper refactoring as this is getting messy.
@cstamas
Copy link
Member Author

cstamas commented Dec 8, 2023

I am about to merge this PR as:

  • it covers all HTTP transports with tests
  • contains substantial bug fixes in two new transports
  • disables the failing tests in new transports for now....

We can always reiterate on this, IMHO the bug fixes are important (and should go into alpha-4)

@cstamas cstamas marked this pull request as ready for review December 8, 2023 17:05
The share test is not an "ordinary" Junit5 test, so
made its method protected.
@cstamas cstamas merged commit 08f102a into apache:master Dec 11, 2023
7 checks passed
@cstamas cstamas deleted the share-test branch December 11, 2023 09:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant