fineract-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [fineract] ptuomola commented on a change in pull request #817: Step 1 of improving SchedulerJobHelper to be more reliable and prevent flaky tests (FINERACT-922)
Date Thu, 21 May 2020 18:16:43 GMT

ptuomola commented on a change in pull request #817:
URL: https://github.com/apache/fineract/pull/817#discussion_r428828283



##########
File path: fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/SchedulerJobHelper.java
##########
@@ -122,45 +139,112 @@ private static String runSchedulerJobAsJSON() {
         return runSchedulerJob;
     }
 
-    public void executeJob(String jobName) throws InterruptedException {
-        List<Map> allSchedulerJobsData = getAllSchedulerJobs();
-        Assert.assertNotNull(allSchedulerJobsData);
-
+    private int getSchedulerJobIdByName(String jobName) {
+        List<Map<String, Object>> allSchedulerJobsData = getAllSchedulerJobs();
         for (Integer jobIndex = 0; jobIndex < allSchedulerJobsData.size(); jobIndex++)
{
             if (allSchedulerJobsData.get(jobIndex).get("displayName").equals(jobName)) {
-                Integer jobId = (Integer) allSchedulerJobsData.get(jobIndex).get("jobId");
+                return (Integer) allSchedulerJobsData.get(jobIndex).get("jobId");
+            }
+        }
+        throw new IllegalArgumentException("No such named Job (see org.apache.fineract.infrastructure.jobs.service.JobName
enum):" + jobName);
+    }
 
-                // Executing Scheduler Job
-                runSchedulerJob(this.requestSpec, jobId.toString());
+    @Deprecated // FINERACT-922 TODO Gradually replace use of this method with new executeAndAwaitJob()
below, if it proves to be more stable than this one
+    public void executeJob(String jobName) throws InterruptedException {
+        // Stop the Scheduler while we manually trigger execution of job, to avoid side effects
and simplify debugging when readings logs
+        updateSchedulerStatus(false);
+
+        int jobId = getSchedulerJobIdByName(jobName);
 
-                // Retrieving Scheduler Job by ID
-                Map schedulerJob = getSchedulerJobById(jobId);
-                Assert.assertNotNull(schedulerJob);
+        // Executing Scheduler Job
+        runSchedulerJob(jobId);
 
-                // Waiting for Job to complete
-                while ((Boolean) schedulerJob.get("currentlyRunning") == true) {
-                    Thread.sleep(15000);
-                    schedulerJob = getSchedulerJobById(jobId);
-                    Assert.assertNotNull(schedulerJob);
-                    System.out.println("Job is Still Running");
-                }
+        // Retrieving Scheduler Job by ID
+        Map<String, Object> schedulerJob = getSchedulerJobById(jobId);
 
-                List<Map> jobHistoryData = getSchedulerJobHistory(jobId);
+        // Waiting for Job to complete
+        while ((Boolean) schedulerJob.get("currentlyRunning") == true) {
+            Thread.sleep(15000);
+            schedulerJob = getSchedulerJobById(jobId);
+            assertNotNull(schedulerJob);
+            System.out.println("Job is Still Running");
+        }
 
-                Assert.assertFalse("Job History is empty :(  Was it too slow? Failures in
background job?", jobHistoryData.isEmpty());
+        List<Map<String, Object>> jobHistoryData = getSchedulerJobHistory(jobId);
 
-                // print error associated with recent job failure (if any)
-                System.out.println("Job run error message (printed only if the job fails:
"
-                        + jobHistoryData.get(jobHistoryData.size() - 1).get("jobRunErrorMessage"));
-                System.out.println("Job failure error log (printed only if the job fails:
"
-                        + jobHistoryData.get(jobHistoryData.size() - 1).get("jobRunErrorLog"));
+        assertFalse("Job History is empty :(  Was it too slow? Failures in background job?",
jobHistoryData.isEmpty());
 
-                // Verifying the Status of the Recently executed Scheduler Job
-                Assert.assertEquals("Verifying Last Scheduler Job Status", "success",
-                        jobHistoryData.get(jobHistoryData.size() - 1).get("status"));
+        // print error associated with recent job failure (if any)
+        System.out.println("Job run error message (printed only if the job fails: "
+                + jobHistoryData.get(jobHistoryData.size() - 1).get("jobRunErrorMessage"));
+        System.out.println("Job failure error log (printed only if the job fails: "
+                + jobHistoryData.get(jobHistoryData.size() - 1).get("jobRunErrorLog"));
+
+        // Verifying the Status of the Recently executed Scheduler Job
+        assertEquals("Verifying Last Scheduler Job Status", "success",
+                jobHistoryData.get(jobHistoryData.size() - 1).get("status"));
+    }
 
-                break;
+    /**
+     * Launches a Job and awaits its completion.
+     * @param jobName displayName (see {@link org.apache.fineract.infrastructure.jobs.service.JobName})
of Scheduler Job
+     *
+     * @author Michael Vorburger.ch
+     */
+    public void executeAndAwaitJob(String jobName) {
+        Duration TIMEOUT = Duration.ofSeconds(30);
+        Duration PAUSE = Duration.ofMillis(500);
+        DateTimeFormatter df = DateTimeFormatter.ISO_INSTANT; // FINERACT-926
+        Instant beforeExecuteTime = now().truncatedTo(ChronoUnit.SECONDS);
+
+        // Stop the Scheduler while we manually trigger execution of job, to avoid side effects
and simplify debugging when readings logs
+        updateSchedulerStatus(false);
+
+        // Executing Scheduler Job
+        int jobId = getSchedulerJobIdByName(jobName);
+        runSchedulerJob(jobId);
+
+        // Await JobDetailData.lastRunHistory [JobDetailHistoryData] jobRunStartTime >=
beforeExecuteTime (or timeout)
+        await().atMost(TIMEOUT).pollInterval(PAUSE).until(jobLastRunHistorySupplier(jobId),
lastRunHistory -> {
+            String jobRunStartText = lastRunHistory.get("jobRunStartTime");
+            if (jobRunStartText == null) {
+                return false;
             }
+            Instant jobRunStartTime = df.parse(jobRunStartText, Instant::from);
+            return jobRunStartTime.equals(jobRunStartTime) || jobRunStartTime.isAfter(beforeExecuteTime);

Review comment:
       @vorburger I think the second check (row 228) is correct as you state (i.e. jobRunEndTime.equals(jobRunStartTime)
|| jobRunEndTime.isAfter(jobRunStartTime))
   
   But the first check I was commenting on (row 217) would still make most sense to me as:
   
   jobRunStartTime.equals(beforeExecuteTime) || jobRunStartTime.isAfter(beforeExecuteTime)
   
   In order to make sure that the job has had a chance to start, before we start looking at
its history. 
   
   If we don't do this and go straight to comparing jobRunStartTime and jobRunEndTime, I think
there's a risk of hitting the same issue as I observed earlier: i.e. that we started looking
for the history before the job even had had a chance to start for this test. 
   
   In such case we would get the previous execution history of the job, jobRunEndTime would
still be >= jobRunStartTime - but this would be the previous run of the job, not the current
one we had triggered.
   
   Or am I missing something here?
   




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



Mime
View raw message