[GitHub] [shardingsphere-elasticjob] wwj-go commented on a change in pull request #1663: Fix OneOffJobBootstrapTest.assertExecute and JobSpringNamespaceWithTypeTest.jobScriptWithJobTypeTest assert failed in an environment of insufficient performance

[GitBox <gi...@apache.org>]

wwj-go commented on a change in pull request #1663:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1663#discussion_r512379975



##########
File path: elasticjob-lite/elasticjob-lite-spring/elasticjob-lite-spring-namespace/src/test/java/org/apache/shardingsphere/elasticjob/lite/spring/namespace/job/JobSpringNamespaceWithTypeTest.java
##########
@@ -35,14 +38,22 @@
     @Resource
     private CoordinatorRegistryCenter regCenter;
     
+    private Scheduler scheduler;
+
     @After
-    public void tearDown() {
-        JobRegistry.getInstance().shutdown(scriptJobName);
+    public void tearDown() throws SchedulerException {
+        while (!scheduler.getCurrentlyExecutingJobs().isEmpty()) {
+            BlockUtils.waitingShortTime();
+        }
+        JobRegistry.getInstance().getJobScheduleController(scriptJobName).shutdown();
     }
     
     @Test
-    public void jobScriptWithJobTypeTest() {
-        BlockUtils.sleep(1000L);
-        assertTrue(regCenter.isExisted("/" + scriptJobName + "/sharding"));

Review comment:
       This condition is not certain. If we wait for 1 second the `regCenter.isExisted("/" + scriptJobName + "/sharding")` will return true in most cases，but in some case it returns false.
   `   BlockUtils.sleep(1000L);` This line make our assert rely on operating system(OS)'s time and scheduler which we should avoid in unit test,  because the behavior of OS is something we cannot decide.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org