JDK-8382118: Refactor open/test/jdk/sun/net/www/protocol/file/DirPermissionDenied.java test#31163
JDK-8382118: Refactor open/test/jdk/sun/net/www/protocol/file/DirPermissionDenied.java test#31163mahendrachhipa wants to merge 2 commits into
Conversation
…issionDenied.java test
|
👋 Welcome back mchhipa! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
|
@mahendrachhipa The following label will be automatically applied to this pull request:
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command. |
Webrevs
|
Marcono1234
left a comment
Marcono1234
left a comment
There was a problem hiding this comment.
These comments are only suggestions; feel free to ignore them if you don't think they are helpful.
| assertThrows(IOException.class, ()-> {URLConnection uc = url.openConnection(); | ||
| uc.connect();}); |
There was a problem hiding this comment.
Might be better to move url.openConnection() out of the assertThrows to ensure that it not unexpectedly throws an exception?
| assertThrows(IOException.class, ()-> {URLConnection uc = url.openConnection(); | ||
| uc.getInputStream();}); |
There was a problem hiding this comment.
Same as above, move url.openConnection() outside of assertThrows?
| assertDoesNotThrow(() -> {URLConnection uc = url.openConnection(); | ||
| uc.getContentLengthLong();}); |
There was a problem hiding this comment.
For better readability, place the statements on separate lines?
| assertDoesNotThrow(() -> {URLConnection uc = url.openConnection(); | |
| uc.getContentLengthLong();}); | |
| assertDoesNotThrow(() -> { | |
| URLConnection uc = url.openConnection(); | |
| uc.getContentLengthLong(); | |
| }); |
|
@mahendrachhipa can you fix the PR title? |
mahendrachhipa
changed the title
3 participants
Breakdown one test tomultiple test each for one method.
Progress
Integration blocker
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/31163/head:pull/31163$ git checkout pull/31163Update a local copy of the PR:
$ git checkout pull/31163$ git pull https://git.openjdk.org/jdk.git pull/31163/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 31163View PR using the GUI difftool:
$ git pr show -t 31163Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/31163.diff
Using Webrev
Link to Webrev Comment