Re: RFR: 8285690: CloneableReference subtest should not throw CloneNotSupportedException [v2]
On Wed, 27 Apr 2022 15:32:35 GMT, Roger Riggs wrote: >> Kim Barrett has updated the pull request incrementally with one additional >> commit since the last revision: >> >> update copyright, @bug list > > LGTM Thanks for reviews @RogerRiggs and @mlchung - PR: https://git.openjdk.java.net/jdk/pull/8418
Re: RFR: 8285690: CloneableReference subtest should not throw CloneNotSupportedException [v2]
On Wed, 27 Apr 2022 18:25:27 GMT, Mandy Chung wrote:
>> That was my initial thought, but it doesn't work - CNSE is a checked
>> exception so must be handled.
>
>> test() and main will need to declare this checked exception.
>
>
> diff --git a/test/jdk/java/lang/ref/ReferenceClone.java
> b/test/jdk/java/lang/ref/ReferenceClone.java
> index bd1ead81bec..2f9386b81e4 100644
> --- a/test/jdk/java/lang/ref/ReferenceClone.java
> +++ b/test/jdk/java/lang/ref/ReferenceClone.java
> @@ -31,12 +31,12 @@ import java.lang.ref.*;
>
> public class ReferenceClone {
> private static final ReferenceQueue QUEUE = new
> ReferenceQueue<>();
> -public static void main(String... args) {
> +public static void main(String... args) throws Exception {
> ReferenceClone refClone = new ReferenceClone();
> refClone.test();
> }
>
> -public void test() {
> +public void test() throws CloneNotSupportedException {
> // test Reference::clone that throws CNSE
> Object o = new Object();
> assertCloneNotSupported(new SoftRef(o));
> @@ -45,9 +45,7 @@ public class ReferenceClone {
>
> // Reference subclass may override the clone method
> CloneableReference ref = new CloneableReference(o);
> -try {
> ref.clone();
> -} catch (CloneNotSupportedException e) {}
> }
>
> private void assertCloneNotSupported(CloneableRef ref) {
> ```
Yes, that would work. But I'd rather keep the code for that subtest close
together, i.e. as currently written.
-
PR: https://git.openjdk.java.net/jdk/pull/8418
Re: RFR: 8285690: CloneableReference subtest should not throw CloneNotSupportedException [v2]
On Wed, 27 Apr 2022 18:19:57 GMT, Kim Barrett wrote:
>> test/jdk/java/lang/ref/ReferenceClone.java line 52:
>>
>>> 50: } catch (CloneNotSupportedException e) {
>>> 51: throw new RuntimeException("CloneableReference::clone
>>> should not throw CloneNotSupportedException");
>>> 52: }
>>
>> Alternatively, it could simply let CNSE propagate.
>>
>>
>> CloneableReference ref = new CloneableReference(o);
>> ref.clone();
>>
>>
>> `test()` and `main` will need to declare this checked exception.
>
> That was my initial thought, but it doesn't work - CNSE is a checked
> exception so must be handled.
> test() and main will need to declare this checked exception.
diff --git a/test/jdk/java/lang/ref/ReferenceClone.java
b/test/jdk/java/lang/ref/ReferenceClone.java
index bd1ead81bec..2f9386b81e4 100644
--- a/test/jdk/java/lang/ref/ReferenceClone.java
+++ b/test/jdk/java/lang/ref/ReferenceClone.java
@@ -31,12 +31,12 @@ import java.lang.ref.*;
public class ReferenceClone {
private static final ReferenceQueue QUEUE = new ReferenceQueue<>();
-public static void main(String... args) {
+public static void main(String... args) throws Exception {
ReferenceClone refClone = new ReferenceClone();
refClone.test();
}
-public void test() {
+public void test() throws CloneNotSupportedException {
// test Reference::clone that throws CNSE
Object o = new Object();
assertCloneNotSupported(new SoftRef(o));
@@ -45,9 +45,7 @@ public class ReferenceClone {
// Reference subclass may override the clone method
CloneableReference ref = new CloneableReference(o);
-try {
ref.clone();
-} catch (CloneNotSupportedException e) {}
}
private void assertCloneNotSupported(CloneableRef ref) {
```
-
PR: https://git.openjdk.java.net/jdk/pull/8418
Re: RFR: 8285690: CloneableReference subtest should not throw CloneNotSupportedException [v2]
On Wed, 27 Apr 2022 15:57:34 GMT, Mandy Chung wrote:
>> Kim Barrett has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> update copyright, @bug list
>
> test/jdk/java/lang/ref/ReferenceClone.java line 52:
>
>> 50: } catch (CloneNotSupportedException e) {
>> 51: throw new RuntimeException("CloneableReference::clone should
>> not throw CloneNotSupportedException");
>> 52: }
>
> Alternatively, it could simply let CNSE propagate.
>
>
> CloneableReference ref = new CloneableReference(o);
> ref.clone();
>
>
> `test()` and `main` will need to declare this checked exception.
That was my initial thought, but it doesn't work - CNSE is a checked exception
so must be handled.
-
PR: https://git.openjdk.java.net/jdk/pull/8418
Re: RFR: 8285690: CloneableReference subtest should not throw CloneNotSupportedException [v2]
On Wed, 27 Apr 2022 10:11:15 GMT, Kim Barrett wrote:
>> Please review this fix to test/jdk/java/lang/ref/ReferenceClone.java. It was
>> passing if CloneableReference::clone were to throw
>> CloneNotSupportedException.
>> That should be a failure.
>>
>> Testing:
>> Locally (linux-x64) verified test still passes. Temporarily changed
>> CloneableReference::clone to throw and verified the expected failure gets
>> reported, unlike before.
>
> Kim Barrett has updated the pull request incrementally with one additional
> commit since the last revision:
>
> update copyright, @bug list
Marked as reviewed by mchung (Reviewer).
test/jdk/java/lang/ref/ReferenceClone.java line 52:
> 50: } catch (CloneNotSupportedException e) {
> 51: throw new RuntimeException("CloneableReference::clone should
> not throw CloneNotSupportedException");
> 52: }
Alternatively, it could simply let CNSE propagate.
CloneableReference ref = new CloneableReference(o);
ref.clone();
`test()` and `main` will need to declare this checked exception.
-
PR: https://git.openjdk.java.net/jdk/pull/8418
Re: RFR: 8285690: CloneableReference subtest should not throw CloneNotSupportedException [v2]
On Wed, 27 Apr 2022 10:11:15 GMT, Kim Barrett wrote: >> Please review this fix to test/jdk/java/lang/ref/ReferenceClone.java. It was >> passing if CloneableReference::clone were to throw >> CloneNotSupportedException. >> That should be a failure. >> >> Testing: >> Locally (linux-x64) verified test still passes. Temporarily changed >> CloneableReference::clone to throw and verified the expected failure gets >> reported, unlike before. > > Kim Barrett has updated the pull request incrementally with one additional > commit since the last revision: > > update copyright, @bug list LGTM - Marked as reviewed by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8418
Re: RFR: 8285690: CloneableReference subtest should not throw CloneNotSupportedException [v2]
> Please review this fix to test/jdk/java/lang/ref/ReferenceClone.java. It was > passing if CloneableReference::clone were to throw CloneNotSupportedException. > That should be a failure. > > Testing: > Locally (linux-x64) verified test still passes. Temporarily changed > CloneableReference::clone to throw and verified the expected failure gets > reported, unlike before. Kim Barrett has updated the pull request incrementally with one additional commit since the last revision: update copyright, @bug list - Changes: - all: https://git.openjdk.java.net/jdk/pull/8418/files - new: https://git.openjdk.java.net/jdk/pull/8418/files/47043626..8f863628 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8418&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8418&range=00-01 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/8418.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8418/head:pull/8418 PR: https://git.openjdk.java.net/jdk/pull/8418
RFR: 8285690: CloneableReference subtest should not throw CloneNotSupportedException
Please review this fix to test/jdk/java/lang/ref/ReferenceClone.java. It was passing if CloneableReference::clone were to throw CloneNotSupportedException. That should be a failure. Testing: Locally (linux-x64) verified test still passes. Temporarily changed CloneableReference::clone to throw and verified the expected failure gets reported, unlike before. - Commit messages: - fail test if CloneableReference throws CloneNotSupportedException Changes: https://git.openjdk.java.net/jdk/pull/8418/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=8418&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8285690 Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8418.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8418/head:pull/8418 PR: https://git.openjdk.java.net/jdk/pull/8418
