P3
This is a strongly related to CODETOOLS-7902065.
TestDescription.equals only compares the *content* of a TestDescription, and not its location (rootRelativePathURL). As part of a FindBugs cleanup long ago, it was noted that TestDescription did not override hashCode. A definition of hashCode was put in place that matches .equals.
That's sort of OK, ... but not OK. In particular, it makes it generally inadvisable/incorrect to use Set<TestDescription> or Map<TestDescription,?> because sometimes conceptually different TestDescriptions will compare .equals. That's because .equals and .hashCode compare the *contents* of the test description, but NOT its *location*. Maybe once upon a time that was a use for those semantics, but it has to be the uncommon case. Normally, I would expect test descriptions in different parts of the test suite to take the position into account.
In an ideal world, I'd wind the clock back and change the name of the functionality currently called .equals.
If we can't change the methods, we should add doc comments to .equals and .hashCode that CLEARLY document what is being compared and STRONGLY ADVISE AGAINST using TestDescription Set or Map, because it will in some cases not behave as expected.
TestDescription.equals only compares the *content* of a TestDescription, and not its location (rootRelativePathURL). As part of a FindBugs cleanup long ago, it was noted that TestDescription did not override hashCode. A definition of hashCode was put in place that matches .equals.
That's sort of OK, ... but not OK. In particular, it makes it generally inadvisable/incorrect to use Set<TestDescription> or Map<TestDescription,?> because sometimes conceptually different TestDescriptions will compare .equals. That's because .equals and .hashCode compare the *contents* of the test description, but NOT its *location*. Maybe once upon a time that was a use for those semantics, but it has to be the uncommon case. Normally, I would expect test descriptions in different parts of the test suite to take the position into account.
In an ideal world, I'd wind the clock back and change the name of the functionality currently called .equals.
If we can't change the methods, we should add doc comments to .equals and .hashCode that CLEARLY document what is being compared and STRONGLY ADVISE AGAINST using TestDescription Set or Map, because it will in some cases not behave as expected.
- relates to
-
CODETOOLS-7902065 com.sun.javatest.TestDescription::equals can return true for different test
-
- Resolved
-