phet commented on code in PR #3944:
URL: https://github.com/apache/gobblin/pull/3944#discussion_r1591963116


##########
gobblin-service/src/test/java/org/apache/gobblin/service/modules/orchestration/OrchestratorTest.java:
##########
@@ -81,12 +84,15 @@ public class OrchestratorTest {
 
   private FlowCatalog flowCatalog;
   private FlowSpec flowSpec;
-  private Orchestrator orchestrator;
+
+  private FlowStatusGenerator mockFlowStatusGenerator;
+  private DagManager mockDagManager;
+  private Orchestrator dagMgrNotFlowLaunchHandlerBasedOrchestrator;
   private static final String TEST_USER = "testUser";
   private static final String TEST_PASSWORD = "testPassword";
   private static final String TEST_TABLE = "quotas";
 
-  @BeforeClass
+  @BeforeMethod

Review Comment:
   I agree and now realize, after you pointed it out, that I didn't cover all 
test init best practice, so much as denounce what's broken about 
`@BeforeClass`-only init.
   
   to encapsulate a fuller picture now, I'd advise
   
   > prefer `@{Before,After}Method` test init to `@{Before,After}Class`".  when 
mocks are in use, the latter w/o the former, MOST LIKELY indicates the wrong 
choice, whereas having only method, but not class-level init is quite fine.  In 
fact the only really acceptable reason for choosing class-level init is to 
amortize payment of initialization tax just once per class, rather than 
incurring once per test case.
   
   at the moment, I can't think of any non-performance-motivated argument to 
use `@BeforeClass` over just adding everything into `@BeforeMethod`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to