On Mon, 16 Nov 2020 17:29:20 GMT, Daniel Fuchs <dfu...@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
