Write tests for recurring tasks controllers and fix a bunch of minor …

…details

- Standardise error messages
- Show the right singular or plural words with counts (eg. 1 job vs. 1 jobs)
- Redirect to tasks list when task is not found instead of root

Also, work around a bug in Active Job's test helper whereby the queue adapter
name is not overridden with the test adapter's name.

app/controllers/concerns/mission_control/jobs/not_found_redirections.rb

@@ -7,7 +7,7 @@ module MissionControl::Jobs::NotFoundRedirections
     end
 
     rescue_from(MissionControl::Jobs::Errors::ResourceNotFound) do |error|
-      redirect_to root_url, alert: error.message
+      redirect_to best_location_for_resource_not_found_error(error), alert: error.message
     end
   end
 
@@ -22,4 +22,12 @@ def best_location_for_job_relation(job_relation)
         root_path
       end
     end
+
+    def best_location_for_resource_not_found_error(error)
+      if error.message.match?(/recurring task/i)
+        application_recurring_tasks_path(@application)
+      else
+        root_url
+      end
+    end
 end

app/views/mission_control/jobs/queues/_queue_title.html.erb

@@ -14,4 +14,4 @@
   </div>
 </h1>
 
-<h2 class="subtitle"><%= queue.size %> pending jobs</h2>
+<h2 class="subtitle"><%= pluralize queue.size, "pending job" %></h2>

app/views/mission_control/jobs/recurring_tasks/show.html.erb

@@ -6,7 +6,7 @@
 <% if @jobs_page.empty? %>
   <%= blank_status_notice "No jobs found for this recurring task" %>
 <% else %>
-  <h2 class="subtitle"><%= @recurring_task.jobs.count %> jobs</h2>
+  <h2 class="subtitle"><%= pluralize @recurring_task.jobs.count, "job" %></h2>
 
   <%= render "mission_control/jobs/shared/jobs", jobs: @jobs_page.jobs %>
 

app/views/mission_control/jobs/workers/show.html.erb

@@ -6,7 +6,7 @@
 <% if @worker.jobs.empty? %>
   <%= blank_status_notice "This worker is idle" %>
 <% else %>
-  <h2 class="subtitle">Running <%= @worker.jobs.size %> jobs</h2>
+  <h2 class="subtitle">Running <%= pluralize @worker.jobs.size, "job" %></h2>
 
   <%= render "mission_control/jobs/shared/jobs", jobs: @worker.jobs %>
 <% end %>

lib/active_job/queue_adapters/solid_queue_ext.rb

@@ -243,7 +243,7 @@ def filter_executions_by_process_id(executions)
           if solid_queue_status.claimed?
             executions.where(process_id: worker_id)
           else
-            raise ActiveJob::Errors::QueryError, "Filtering by worker ID is not supported for status #{jobs_relation.status}"
+            raise ActiveJob::Errors::QueryError, "Filtering by worker id is not supported for status #{jobs_relation.status}"
           end
         end
 

lib/mission_control/jobs/server/recurring_tasks.rb

@@ -9,7 +9,7 @@ def find_recurring_task(task_id)
     if task = queue_adapter.find_recurring_task(task_id)
       MissionControl::Jobs::RecurringTask.new(queue_adapter: queue_adapter, **task)
     else
-      raise MissionControl::Jobs::Errors::ResourceNotFound, "No recurring task found with ID #{task_id}"
+      raise MissionControl::Jobs::Errors::ResourceNotFound, "Recurring task with id '#{task_id}' not found"
     end
   end
 end

lib/mission_control/jobs/server/workers.rb

@@ -9,7 +9,7 @@ def find_worker(worker_id)
     if worker = queue_adapter.find_worker(worker_id)
       MissionControl::Jobs::Worker.new(queue_adapter: queue_adapter, **worker)
     else
-      raise MissionControl::Jobs::Errors::ResourceNotFound, "No worker found with ID #{worker_id}"
+      raise MissionControl::Jobs::Errors::ResourceNotFound, "Worker with id '#{worker_id}' not found"
     end
   end
 end

test/controllers/jobs_controller_test.rb

@@ -26,8 +26,7 @@ class MissionControl::Jobs::JobsControllerTest < ActionDispatch::IntegrationTest
   test "get jobs and job details when there are multiple instances of the same job due to automatic retries" do
     job = AutoRetryingJob.perform_later
 
-    # Wait until the job has been executed and retried
-    perform_enqueued_jobs_async { sleep(1) }
+    perform_enqueued_jobs_async
 
     get mission_control_jobs.application_jobs_url(@application, :finished)
     assert_response :ok

test/controllers/recurring_tasks_controller_test.rb

@@ -0,0 +1,51 @@
+require "test_helper"
+
+class MissionControl::Jobs::RecurringTasksControllerTest < ActionDispatch::IntegrationTest
+  setup do
+    # Work around a bug in Active Job's test helpers, whereby the test adapter is returned
+    # when it's set, but the queue adapter name remains to be the previous adapter, bypassing
+    # the set test adapter. This can be removed once the bug is fixed in Active Job
+    PauseJob.queue_adapter = :solid_queue
+  end
+
+  teardown do
+    PauseJob.queue_adapter = :resque
+  end
+
+  test "get recurring task list" do
+    dispatch_jobs_async(wait: 2.seconds) do
+      get mission_control_jobs.application_recurring_tasks_url(@application)
+      assert_response :ok
+
+      assert_select "tr.recurring_task", 1
+      assert_select "td a", "periodic_pause_job"
+      assert_select "td a", "PauseJob"
+      assert_select "td", "every second"
+      assert_select "td", /less than \d+ seconds ago/
+    end
+  end
+
+  test "get recurring task details and job list" do
+    dispatch_jobs_async(wait: 1.seconds) do
+      get mission_control_jobs.application_recurring_task_url(@application, "periodic_pause_job")
+      assert_response :ok
+
+      assert_select "h1", /periodic_pause_job/
+      assert_select "h2", "1 job"
+      assert_select "tr.job", 1
+      assert_select "td a", "PauseJob"
+      assert_select "td div", /Enqueued less than a minute ago/
+    end
+  end
+
+  test "redirect to recurring tasks list when recurring task doesn't exist" do
+    dispatch_jobs_async do
+      get mission_control_jobs.application_recurring_task_url(@application, "invalid_key")
+      assert_redirected_to mission_control_jobs.application_recurring_tasks_url(@application)
+
+      follow_redirect!
+
+      assert_select "article.is-danger", /Recurring task with id 'invalid_key' not found/
+    end
+  end
+end

test/controllers/retries_controller_test.rb

@@ -12,8 +12,7 @@ class MissionControl::Jobs::JobsControllerTest < ActionDispatch::IntegrationTest
   test "retry jobs when there are multiple instances of the same job due to automatic retries" do
     job = AutoRetryingJob.perform_later
 
-    # Wait until the job has been executed and retried
-    perform_enqueued_jobs_async { sleep(1) }
+    perform_enqueued_jobs_async
 
     get mission_control_jobs.application_jobs_url(@application, :failed)
     assert_response :ok

test/controllers/workers_controller_test.rb

@@ -7,24 +7,26 @@ class MissionControl::Jobs::WorkersControllerTest < ActionDispatch::IntegrationT
   end
 
   test "get workers" do
-    perform_enqueued_jobs_async
-    worker = @server.workers.first
+    perform_enqueued_jobs_async(wait: 0) do
+      worker = @server.workers.first
 
-    get mission_control_jobs.application_workers_url(@application)
+      get mission_control_jobs.application_workers_url(@application)
 
-    assert_includes response.body, "worker #{worker.id}"
-    assert_includes response.body, "PauseJob"
-    assert_includes response.body, "my-hostname-123"
+      assert_includes response.body, "worker #{worker.id}"
+      assert_includes response.body, "PauseJob"
+      assert_includes response.body, "my-hostname-123"
+    end
   end
 
   test "get worker details" do
-    perform_enqueued_jobs_async
-    worker = @server.workers.first
+    perform_enqueued_jobs_async(wait: 0) do
+      worker = @server.workers.first
 
-    get mission_control_jobs.application_worker_url(@application, worker.id)
-    assert_response :ok
+      get mission_control_jobs.application_worker_url(@application, worker.id)
+      assert_response :ok
 
-    assert_includes response.body, "Worker #{worker.id}"
-    assert_includes response.body, "Running 2 jobs"
+      assert_includes response.body, "Worker #{worker.id}"
+      assert_includes response.body, "Running 2 jobs"
+    end
   end
 end

test/test_helper.rb

@@ -79,22 +79,34 @@ class ActionDispatch::IntegrationTest
     @application = MissionControl::Jobs.applications["integration-tests"]
     @server = @application.servers[:solid_queue]
     @worker = SolidQueue::Worker.new(queues: "*", threads: 2, polling_interval: 0.01)
+
+    recurring_task = { periodic_pause_job: { class: "PauseJob", schedule: "every second" } }
+    @dispatcher = SolidQueue::Dispatcher.new(recurring_tasks: recurring_task)
   end
 
   teardown do
     @worker.stop
+    @dispatcher.stop
   end
 
   private
     def queue_adapter_for_test
       ActiveJob::QueueAdapters::SolidQueueAdapter.new
     end
 
-    def perform_enqueued_jobs_async
+    def perform_enqueued_jobs_async(wait: 1.second)
       @worker.start
-      if block_given?
-        yield
-        @worker.stop
-      end
+      sleep(wait)
+
+      yield if block_given?
+      @worker.stop
+    end
+
+    def dispatch_jobs_async(wait: 1.second)
+      @dispatcher.start
+      sleep(wait)
+
+      yield if block_given?
+      @dispatcher.stop
     end
 end