RFR: 8252304: Seed an HttpRequest.Builder from an existing HttpRequest [v8]

Patrick Concannon pconcannon at openjdk.java.net
Tue Nov 17 16:20:10 UTC 2020


On Mon, 16 Nov 2020 17:29:20 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

>> Patrick Concannon has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 15 additional commits since the last revision:
>> 
>>  - 8252304: Added more test cases to check header filter
>>  - Merge remote-tracking branch 'origin/master' into JDK-8252304
>>  - Merge remote-tracking branch 'origin/master' into JDK-8252304
>>  - 8252304: BiPredicate parameter added to newBuilder
>>  - Merge remote-tracking branch 'origin/master' into JDK-8252304
>>  - Merge remote-tracking branch 'origin/master' into JDK-8252304
>>  - Merge remote-tracking branch 'origin/master' into JDK-8252304
>>  - Merge remote-tracking branch 'origin/master' into JDK-8252304
>>  - Merge remote-tracking branch 'origin/master' into JDK-8252304
>>  - 8252304: Removed catch block from newBuilder(HttpRequest)
>>  - ... and 5 more: https://git.openjdk.java.net/jdk/compare/e47c5eef...03b13e7a
>
> test/jdk/java/net/httpclient/HttpRequestNewBuilderTest.java line 320:
> 
>> 318:     @Test(dataProvider = "testRequests")
>> 319:     public void testRemoveHeader(HttpRequest request) {
>> 320:         BiPredicate<String, String> filter = (n, v) -> !n.equalsIgnoreCase("testName1");
> 
> This assumes that there is at least one `request` such that `request.headers().firstValue("testName1").isPresent()` otherwise the test will trivially pass.
> I wonder if we need a specific dataProvider here, so that we can assert:
> 
> assertTrue(request.headers().firstValue("testName1").isPresent());
> 
> first thing in the test? That - or have an AtomicInteger that counts how many time the original request has the conf we want to remove, and then assert that the counter > 0 in an `@AfterClass` method?

Would it be enough to check in each test case that ensures if we are examining a request with headers, it contains at least one called `testName1`? i.e.
        if(!request.headers().map().isEmpty())
            assertTrue(request.headers().map().containsKey("testName1"));

-------------

PR: https://git.openjdk.java.net/jdk/pull/1059


More information about the net-dev mailing list