> On Oct 12, 2020, at 3:25 PM, Per Liden <pli...@openjdk.java.net> wrote:
>
> On Mon, 12 Oct 2020 15:56:59 GMT, Kim Barrett <kbarr...@openjdk.org> wrote:
>
>>> test/hotspot/jtreg/gc/TestReferenceRefersTo.java line 186:
>>>
>>>> 184:
>>>> 185: expectNotValue(testWeak2, testObject3, "testWeak2");
>>>> 186: expectValue(testWeak3, testObject3, "testWeak3");
>>>
>>> It looks to me like the `expectNotCleared()` and `expectNotValue()` methods
>>> can be removed. All expect-tests can be
>>> done with just `expectCleared()` and `expectValue()`.
>>
>> I don't think so. For example,
>> `expectNotCleared(testWeak1, "testWeak1")`
>> is not testing the same thing as
>> `expectValue(testWeak1, testObject1, "testWeak1")`
>> If the implementation is correct they will indeed always be consistent. But
>> they could be different with an incorrect
>> implementation.
>
> Doesn't that just depend on how you implement
> `expectValue()`/`expectCleared()`? You could for example implement
> `expectValue()`/`expectCleared()` to be supersets of
> `expectNotCleared()`/`expectNotValue()`, which I think would make
> this easier to read. Something like this:
>
> private static void expectValue(...) {
> if (ref.refersTo(null)) {
> fail("expected " + which + " to not be cleared");
> }
>
> if (!ref.refersTo(value)) {
> fail(which + " refers to unexpected value");
> }
> }
>
> private static void expectCleared(...) {
> if (ref.refersTo(new TestObject(5))) {
> fail(which + " refers to unexpected value");
> }
>
> if (!ref.refersTo(null)) {
> fail("expected " + which + " to be cleared");
> }
> }
I didn't understand what you were previously proposing. Now I think I see.
I've demoted expectNotValue to a helper for the other expectMumble
functions. This allowed cleaning up the checking blocks, so each one makes
one call to an expectMumble for each test reference.
However, expectNotCleared can't be eliminated in favor of just using
expectValue. In the places where expectNotCleared is used, the expected
value is unavailable; the associated testObjectN variable has been nulled.
