On Mon, 7 Sep 2020 09:18:31 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Can I please get a review and a sponsor for a fix for the issue reported at >> https://bugs.openjdk.java.net/browse/JDK-8252767? >> As noted in that issue, the `sun.net.www.URLConnection#setRequestProperty` >> is throwing a `IllegalAccessError` instead >> of a `IllegalStateException`. The commit here fixes that and includes a test >> which reproduces the issue and verifies >> the fix. Would a CSR be needed for this change? > > Jaikiran Pai has refreshed the contents of this pull request, and previous > commits have been removed. The incremental > views will show differences compared to the previous content of the PR. The > pull request contains one new commit since > the last revision: > 8252767: URLConnection.setRequestProperty throws IllegalAccessError > Summary: Throw an IllegalStateException from > sun.net.www.URLConnection#setRequestProperty when already connected > Reviewed-By: test/jdk/java/net/URLConnection/RequestProperties.java line 159: > 157: // expected > 158: } > 159: } Given that the test update is significant then it makes me wonder if we should just move to TestNG while we're at it. It would avoid needing to add expectIllegalStateException. Minor inconsistency is that is the existing test now has "NPE" as the suffix, the new test "IllegalStateException" as the suffix. Also can we find a better name for "unmetExpectationErrorMessage" as it's not clear to maintainers what "unmet" means. ------------- PR: https://git.openjdk.java.net/jdk/pull/26
