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 
contextualize overall test init best practice, so much as denounce what's 
broken about using solely `@BeforeClass` 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 most common acceptable rationale for 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