Uploaded image for project: 'Code Tools'
  1. Code Tools
  2. CODETOOLS-7902343

TestDescription should document or unimplement equals/hashCode

    Details

    • Type: Bug
    • Status: New
    • Priority: P3
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: tools
    • Labels:
      None

      Description

      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.

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                dbessono Dmitry Bessonov
                Reporter:
                jjg Jonathan Gibbons
              • Votes:
                0 Vote for this issue
                Watchers:
                1 Start watching this issue

                Dates

                • Created:
                  Updated: