Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

...

  • JUnit Tests
    • Test file naming
      • WIP
      • "Test" prefix
    • Where appropriate, use assertj instead of try/catch blocks.  Try/catch blocks are notoriously incorrect or incorrectly maintained, as developers frequently forget to add "fail()" calls in the appropriate branch.
    • WIP
  • Use of Static Methods
    • In general, methods should not be declared "static" unless they are side-effect free.  For instance, they should not interact with external systems (e.g., DBMS).
    • Exceptions include "registration" methods, which are used to register objects once, typically at start-up.  Loggers fall into this category.
    • This is intended to be a guideline and not a hard and fast rule, though exceptions should be granted sparingly.  Typically, a static method can be easily converted into a regular method.
    • Rationale with respect to testing:
      • Testing rainy day scenarios: It’s much easier to test rainy day scenarios if you can force an invoked method to throw an exception or otherwise return or modify values in some particular way.  This is easier to accomplish if the method can be overridden.
        • If the static method is in another class then powermock can be used to do this.  If not, then it becomes problematic.  This is true even when dealing with static methods that are side-effect free.
      • Amount of configuration that must be done: To execute code that’s held together by static methods, sometimes lots of supporting classes have to be configured, when the use of mocks or partially configured objects would suffice if non-static methods were used.
      • Starting tests fresh: When static methods manipulate static data, every test that uses those static methods, whether directly or indirectly, has to be aware that the static data may not have been left in a desired state.  Typically, a mechanism must be added to re-initialize such data.
      • Ease of running multiple objects in separate threads: When regular methods are used, it is typically quite easy to spin up multiple threads, each with its own object, when some type of multi-threaded testing must be done.  That is often much more difficult if the objects use static methods that manipulate static data.
      • Multiple types of junit tests: The first two, and sometimes the latter, as well, are made much easier using objects with regular methods.
        • Individual classes/methods
        • Code coverage
        • Component tests

Committer Strategy

  • Require 3 committers to +1 a review - at least 2 companies represented. 
  • Require at least one expert of the repo to +1 (may or may not be a committer).
  • For substantial reviews, either size or logic changes, then time limit of 24 hours to review can override.
  • A +2 from committer or PTL, one can imply they are good with any future patches for a review.