andreaslind01 commented on PR #6742: URL: https://github.com/apache/jmeter/pull/6742#issuecomment-5263786509
> Thanks @andreaslind01 — manual testing against the built distribution (browsing-style scenario, HTTP/2, same idle-gap pattern as the original repro) is conclusive: the stale-connection failure is gone. Great fix, and a genuinely clever regression test. > > Two smaller, non-blocking points from going through the GUI while testing: > > ### 1. The HTTP Version combo doesn't reflect what each implementation actually supports > In _HTTP Request_ → _Advanced_ (and _HTTP Request Defaults_), the _Implementation_ combo offers `HttpClient4` / `HttpClient5` / `Java` / (default), and _HTTP Version_ offers `HTTP/1.1` / `HTTP/2` / (empty) — independently of each other. `HttpClient4` and `HTTP/2` can both be selected together, but `HTTPHC4Impl` never reads the `httpVersion` property at all, so the request silently runs as HTTP/1.1 regardless — nothing in the UI signals that the combination is a no-op. > > Would be worth either: > > * disabling/graying out `HTTP/2` in the _HTTP Version_ combo when `HttpClient4` is the selected implementation, and/or > * relabeling the item to make the fallback explicit, e.g. `HTTP/2.0 (back to HTTP/1.1)` when the active implementation doesn't support it. > > Relevant: `httpImplementation`/`httpVersion` combos in `HttpTestSampleGui.java` and `HttpDefaultsGui.java` — currently just two independent `JComboBox`es with no listener tying one to the other's state/labels. > > ### 2. Advanced-tab screenshot is stale > `xdocs/images/screenshots/http-request-advanced-tab.png` (referenced from `component_reference.xml`) predates this PR by several years and doesn't show the new _HTTP Version_ field or the `HttpClient5` implementation choice. Worth refreshing it as part of this PR (ideally with the Metal look-and-feel, to match the rest of the JMeter docs' screenshots) so the manual covers the feature it now documents. Thanks for the testing and the detailed review @milamberspace. Regarding the HTTP Version selector: I actually tend towards removing it rather than introducing implementation-specific enable/disable logic. The selected implementation already determines the effective protocol behavior (HttpClient4 → HTTP/1.1, HttpClient5/Java → HTTP/2 with HTTP/1.1 fallback). A tooltip explaining this behavior would likely be clearer than allowing users to choose combinations that are effectively ignored. For the few cases where HTTP/1.1 must be enforced, I would prefer dedicated properties in jmeter.properties rather than additional GUI complexity. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
