On Wed, 8 Feb 2023 09:39:21 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Alan Bateman has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains five commits: >> >> - Merge >> - Merge >> - Fix typos in comments >> - GetStackTrace.java test missing @requires vm.continuations >> - Initial commit > > test/jdk/java/lang/Thread/virtual/HoldsLock.java line 131: > >> 129: assertEquals(vthread.getClass().getName(), >> info.getLockInfo().getClassName()); >> 130: assertTrue(info.getLockInfo().getIdentityHashCode() == >> System.identityHashCode(vthread)); >> 131: assertTrue(info.getLockOwnerId() == vthreadId); > > Previously, before the change to these lines, in case of failure, the > `assertEquals` would have printed even the incorrect/unexpected value in the > log file/exception. Now with the usage of `assertTrue`, that information is > lost from the failures. Do you think, in the context of this test, it would > be worth to continue using assertEquals and just switch the param order to > match junit expectations. We will be coming back to this test in the future (part of it is disabled) so we can re-visit this test at that time. > Perhaps `assertNull(name.get())`?. Same on line 325. Okay, we can do that, note that it impacts 45+ usages and I had hoped to avoid changing the tests too much. ------------- PR: https://git.openjdk.org/jdk/pull/12426
