----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69657/#review211648 -----------------------------------------------------------
The patch does a great job into the right direction. Only a few nit-like comments: * test method naming * separate out test classes like `TestWorkflowPurgeXCommand`, `TestCoordinatorPurgeXCommand`, `TestBundlePurgeXCommand` * apply e.g. the builder pattern to set up test bed at the beginning of each test method / at the end when it comes to assertions * separate out calls to `new PurgeXCommand(...).call()` with descriptive method names core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 82 (original), 90 (patched) <https://reviews.apache.org/r/69657/#comment297052> Is it absolutely necessary to have yet another `Services` reference? Wasn't the existing `protected Services services` enough / working properly? core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 90 (original), 98 (patched) <https://reviews.apache.org/r/69657/#comment297053> Is it absolutely necessary to have yet another `Services` reference? Wasn't the existing `protected Services services` enough / working properly? core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 103 (original), 111 (patched) <https://reviews.apache.org/r/69657/#comment297054> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 144 (original), 126 (patched) <https://reviews.apache.org/r/69657/#comment297055> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 185 (original), 141 (patched) <https://reviews.apache.org/r/69657/#comment297056> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 226 (original), 156 (patched) <https://reviews.apache.org/r/69657/#comment297057> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 266 (original), 171 (patched) <https://reviews.apache.org/r/69657/#comment297058> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 306 (original), 187 (patched) <https://reviews.apache.org/r/69657/#comment297059> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 346 (original), 203 (patched) <https://reviews.apache.org/r/69657/#comment297060> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 386 (original), 219 (patched) <https://reviews.apache.org/r/69657/#comment297061> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 423 (original), 235 (patched) <https://reviews.apache.org/r/69657/#comment297062> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 476 (original), 257 (patched) <https://reviews.apache.org/r/69657/#comment297063> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 529 (original), 280 (patched) <https://reviews.apache.org/r/69657/#comment297064> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 582 (original), 302 (patched) <https://reviews.apache.org/r/69657/#comment297065> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 631 (original), 324 (patched) <https://reviews.apache.org/r/69657/#comment297066> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 693 (original), 347 (patched) <https://reviews.apache.org/r/69657/#comment297067> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 758 (original), 370 (patched) <https://reviews.apache.org/r/69657/#comment297068> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 818 (original), 391 (patched) <https://reviews.apache.org/r/69657/#comment297069> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 879 (original), 413 (patched) <https://reviews.apache.org/r/69657/#comment297070> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 1075 (original), 439 (patched) <https://reviews.apache.org/r/69657/#comment297071> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 1139 (original), 459 (patched) <https://reviews.apache.org/r/69657/#comment297072> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 1351 (original), 484 (patched) <https://reviews.apache.org/r/69657/#comment297073> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 1434 (original), 510 (patched) <https://reviews.apache.org/r/69657/#comment297074> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 1519 (original), 537 (patched) <https://reviews.apache.org/r/69657/#comment297075> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Lines 573 (patched) <https://reviews.apache.org/r/69657/#comment297076> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Lines 599 (patched) <https://reviews.apache.org/r/69657/#comment297077> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 2274 (original), 633 (patched) <https://reviews.apache.org/r/69657/#comment297078> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 2335 (original), 653 (patched) <https://reviews.apache.org/r/69657/#comment297079> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 2396 (original), 674 (patched) <https://reviews.apache.org/r/69657/#comment297080> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 2500 (original), 700 (patched) <https://reviews.apache.org/r/69657/#comment297081> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Lines 721 (patched) <https://reviews.apache.org/r/69657/#comment297082> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 2871 (original), 924 (patched) <https://reviews.apache.org/r/69657/#comment297083> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 2902 (original), 951 (patched) <https://reviews.apache.org/r/69657/#comment297084> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 2933 (original), 978 (patched) <https://reviews.apache.org/r/69657/#comment297085> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 3006 (original), 1010 (patched) <https://reviews.apache.org/r/69657/#comment297086> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 3066 (original), 1034 (patched) <https://reviews.apache.org/r/69657/#comment297087> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 3150 (original), 1061 (patched) <https://reviews.apache.org/r/69657/#comment297088> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 3234 (original), 1088 (patched) <https://reviews.apache.org/r/69657/#comment297089> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 3328 (original), 1118 (patched) <https://reviews.apache.org/r/69657/#comment297090> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 3433 (original), 1155 (patched) <https://reviews.apache.org/r/69657/#comment297091> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 3540 (original), 1187 (patched) <https://reviews.apache.org/r/69657/#comment297092> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 3648 (original), 1219 (patched) <https://reviews.apache.org/r/69657/#comment297093> Please provide a more descriptive name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Line 3762 (original), 1249 (patched) <https://reviews.apache.org/r/69657/#comment297095> Please provide a more descriptive name. - András Piros On Jan. 3, 2019, 8:12 a.m., Andras Salamon wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69657/ > ----------------------------------------------------------- > > (Updated Jan. 3, 2019, 8:12 a.m.) > > > Review request for oozie, András Piros and Kinga Marton. > > > Repository: oozie-git > > > Description > ------- > > OOZIE-3407 - Cleanup TestPurgeXCommand > > > Diffs > ----- > > core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java > 107547d87d294d600986d361ea1a6f92298e97f7 > > > Diff: https://reviews.apache.org/r/69657/diff/1/ > > > Testing > ------- > > Running unit tests localy > > > Thanks, > > Andras Salamon > >
