Conditions | 1 |
Paths | 1 |
Total Lines | 116 |
Code Lines | 69 |
Lines | 0 |
Ratio | 0 % |
Changes | 0 |
Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.
For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.
Commonly applied refactorings include:
If many parameters/temporary variables are present:
1 | <?php |
||
276 | public function testJobHealthCheck() |
||
277 | { |
||
278 | // Create a job and add it to the queue |
||
279 | $svc = $this->getService(); |
||
280 | $logger = $svc->getLogger(); |
||
281 | $job = new TestQueuedJob(QueuedJob::IMMEDIATE); |
||
282 | $job->firstJob = true; |
||
283 | $id = $svc->queueJob($job); |
||
284 | $descriptor = QueuedJobDescriptor::get()->byID($id); |
||
285 | |||
286 | // Verify initial state is new and LastProcessedCount is not marked yet |
||
287 | $this->assertEquals(QueuedJob::STATUS_NEW, $descriptor->JobStatus); |
||
288 | $this->assertEquals(0, $descriptor->StepsProcessed); |
||
289 | $this->assertEquals(-1, $descriptor->LastProcessedCount); |
||
290 | $this->assertEquals(0, $descriptor->ResumeCounts); |
||
291 | |||
292 | // Loop 1 - Pick up new job and attempt to run it |
||
293 | // Job health should not attempt to cleanup unstarted jobs |
||
294 | $svc->checkJobHealth(QueuedJob::IMMEDIATE); |
||
295 | $nextJob = $svc->getNextPendingJob(QueuedJob::IMMEDIATE); |
||
296 | |||
297 | // Ensure that this is the next job ready to go |
||
298 | $descriptor = QueuedJobDescriptor::get()->byID($id); |
||
299 | $this->assertEquals($nextJob->ID, $descriptor->ID); |
||
300 | $this->assertEquals(QueuedJob::STATUS_NEW, $descriptor->JobStatus); |
||
301 | $this->assertEquals(0, $descriptor->StepsProcessed); |
||
302 | $this->assertEquals(-1, $descriptor->LastProcessedCount); |
||
303 | $this->assertEquals(0, $descriptor->ResumeCounts); |
||
304 | |||
305 | // Run 1 - Start the job (no work is done) |
||
306 | $descriptor->JobStatus = QueuedJob::STATUS_INIT; |
||
307 | $descriptor->write(); |
||
308 | |||
309 | // Assume that something bad happens at this point, the process dies during execution, and |
||
310 | // the task is re-initiated somewhere down the track |
||
311 | |||
312 | // Loop 2 - Detect broken job, and mark it for future checking. |
||
313 | $svc->checkJobHealth(QueuedJob::IMMEDIATE); |
||
314 | $nextJob = $svc->getNextPendingJob(QueuedJob::IMMEDIATE); |
||
315 | |||
316 | // Note that we don't immediately try to restart it until StepsProcessed = LastProcessedCount |
||
317 | $descriptor = QueuedJobDescriptor::get()->byID($id); |
||
318 | $this->assertFalse($nextJob); // Don't run it this round please! |
||
319 | $this->assertEquals(QueuedJob::STATUS_INIT, $descriptor->JobStatus); |
||
320 | $this->assertEquals(0, $descriptor->StepsProcessed); |
||
321 | $this->assertEquals(0, $descriptor->LastProcessedCount); |
||
322 | $this->assertEquals(0, $descriptor->ResumeCounts); |
||
323 | |||
324 | // Loop 3 - We've previously marked this job as broken, so restart it this round |
||
325 | // If no more work has been done on the job at this point, assume that we are able to |
||
326 | // restart it |
||
327 | $logger->clear(); |
||
328 | $svc->checkJobHealth(QueuedJob::IMMEDIATE); |
||
329 | $nextJob = $svc->getNextPendingJob(QueuedJob::IMMEDIATE); |
||
330 | |||
331 | // This job is resumed and exeuction is attempted this round |
||
332 | $descriptor = QueuedJobDescriptor::get()->byID($id); |
||
333 | $this->assertEquals($nextJob->ID, $descriptor->ID); |
||
334 | $this->assertEquals(QueuedJob::STATUS_WAIT, $descriptor->JobStatus); |
||
335 | $this->assertEquals(0, $descriptor->StepsProcessed); |
||
336 | $this->assertEquals(0, $descriptor->LastProcessedCount); |
||
337 | $this->assertEquals(1, $descriptor->ResumeCounts); |
||
338 | $this->assertContains('A job named A Test job appears to have stalled. It will be stopped and restarted, please login to make sure it has continued', $logger->getMessages()); |
||
339 | |||
340 | // Run 2 - First restart (work is done) |
||
341 | $descriptor->JobStatus = QueuedJob::STATUS_RUN; |
||
342 | $descriptor->StepsProcessed++; // Essentially delays the next restart by 1 loop |
||
343 | $descriptor->write(); |
||
344 | |||
345 | // Once again, at this point, assume the job fails and crashes |
||
346 | |||
347 | // Loop 4 - Assuming a job has LastProcessedCount < StepsProcessed we are in the same |
||
348 | // situation as step 2. |
||
349 | // Because the last time the loop ran, StepsProcessed was incremented, |
||
350 | // this indicates that it's likely that another task could be working on this job, so |
||
351 | // don't run this. |
||
352 | $svc->checkJobHealth(QueuedJob::IMMEDIATE); |
||
353 | $nextJob = $svc->getNextPendingJob(QueuedJob::IMMEDIATE); |
||
354 | |||
355 | $descriptor = QueuedJobDescriptor::get()->byID($id); |
||
356 | $this->assertFalse($nextJob); // Don't run jobs we aren't sure should be restarted |
||
357 | $this->assertEquals(QueuedJob::STATUS_RUN, $descriptor->JobStatus); |
||
358 | $this->assertEquals(1, $descriptor->StepsProcessed); |
||
359 | $this->assertEquals(1, $descriptor->LastProcessedCount); |
||
360 | $this->assertEquals(1, $descriptor->ResumeCounts); |
||
361 | |||
362 | // Loop 5 - Job is again found to not have been restarted since last iteration, so perform second |
||
363 | // restart. The job should be attempted to run this loop |
||
364 | $logger->clear(); |
||
365 | $svc->checkJobHealth(QueuedJob::IMMEDIATE); |
||
366 | $nextJob = $svc->getNextPendingJob(QueuedJob::IMMEDIATE); |
||
367 | |||
368 | // This job is resumed and exeuction is attempted this round |
||
369 | $descriptor = QueuedJobDescriptor::get()->byID($id); |
||
370 | $this->assertEquals($nextJob->ID, $descriptor->ID); |
||
371 | $this->assertEquals(QueuedJob::STATUS_WAIT, $descriptor->JobStatus); |
||
372 | $this->assertEquals(1, $descriptor->StepsProcessed); |
||
373 | $this->assertEquals(1, $descriptor->LastProcessedCount); |
||
374 | $this->assertEquals(2, $descriptor->ResumeCounts); |
||
375 | $this->assertContains('A job named A Test job appears to have stalled. It will be stopped and restarted, please login to make sure it has continued', $logger->getMessages()); |
||
376 | |||
377 | // Run 3 - Second and last restart (no work is done) |
||
378 | $descriptor->JobStatus = QueuedJob::STATUS_RUN; |
||
379 | $descriptor->write(); |
||
380 | |||
381 | // Loop 6 - As no progress has been made since loop 3, we can mark this as dead |
||
382 | $logger->clear(); |
||
383 | $svc->checkJobHealth(QueuedJob::IMMEDIATE); |
||
384 | $nextJob = $svc->getNextPendingJob(QueuedJob::IMMEDIATE); |
||
385 | |||
386 | // Since no StepsProcessed has been done, don't wait another loop to mark this as dead |
||
387 | $descriptor = QueuedJobDescriptor::get()->byID($id); |
||
388 | $this->assertEquals(QueuedJob::STATUS_PAUSED, $descriptor->JobStatus); |
||
389 | $this->assertEmpty($nextJob); |
||
390 | $this->assertContains('A job named A Test job appears to have stalled. It has been paused, please login to check it', $logger->getMessages()); |
||
391 | } |
||
392 | } |
||
393 |
You can fix this by adding a namespace to your class:
When choosing a vendor namespace, try to pick something that is not too generic to avoid conflicts with other libraries.