[tor-bugs] #22733 [Metrics/Library]: Use parameterized tests instead of repeated methods

[Tor Bug Tracker & Wiki]

#22733: Use parameterized tests instead of repeated methods
-----------------------------+------------------------------
 Reporter:  iwakeh           |          Owner:  metrics-team
     Type:  enhancement      |         Status:  new
 Priority:  Medium           |      Milestone:
Component:  Metrics/Library  |        Version:
 Severity:  Normal           |     Resolution:
 Keywords:  metrics-2018     |  Actual Points:
Parent ID:                   |         Points:
 Reviewer:                   |        Sponsor:
-----------------------------+------------------------------

Comment (by iwakeh):

 Replying to [comment:4 karsten]:
 > I mostly agree that parameterized tests make the code more readable.
 However, there's one thing that makes them a bit less readable: without a
 test method name, whoever reads the code needs to guess why given test
 input produces given test output. That's not always obvious just by
 looking at the data.

 Well, the comments/descriptions can easily be added to the test data.

 >
 > As a (truly) random example, look at the various `testSearchBase64*`
 methods
 [https://gitweb.torproject.org/onionoo.git/tree/src/test/java/org/torproject/onionoo/server/ResourceServletTest.java#n633
 here]. And now imagine how this would look like without test method names.
 Sure, it would be shorter. But would you still understand that the input
 data for `testSearchBase64HashedFingerprintTorkaZ` is the hashed
 fingerprint of one of the example relays? Stated differently, from looking
 at the data alone, would you be able to tell whether a given test case is
 already covered by existing tests or not?

 The methods names hint about the test#s purpose, but are not really very
 helpful for the outside reader, being one word camel case.

 >
 > I guess when we move to parameterized tests we should try to preserve
 the information that is currently contained in test method names. It could
 be in Java comments, though they might not be used or updated in practice.
 It could also be a String in yet another parameter (maybe first?) which
 might be printed out by JUnit in case of a failing test. Are there best
 practices for this issue, or how do others solve this?

 (answered above, ad the description string to the data)

 >
 > Another potential problem is that, AFAIU, we'll have to split each (or
 at least most) of the current test classes into one or more that share
 parameters and another one that is non-parameterized. Should be okay, but
 we should ideally have a plan for this before moving around all existing
 unit test classes.

 This is a valid point and will take some time structuring the changes.
 One immediate improvement would be adding a test description to the
 `@Test` annotation.  These could be re-used later.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/22733#comment:5>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online