Skip to content

Commit

Permalink
Use default line length for cop
Browse files Browse the repository at this point in the history
  • Loading branch information
adrianna-chang-shopify committed Sep 13, 2022
1 parent 77184ff commit 4dfd9d6
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 63 deletions.
3 changes: 0 additions & 3 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,6 @@ AllCops:
Bundler/OrderedGems:
Enabled: true

Layout/LineLength:
Max: 80

Style/Documentation:
Enabled: true
Include:
Expand Down
3 changes: 1 addition & 2 deletions app/controllers/maintenance_tasks/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@ class ApplicationController < ActionController::Base
end

before_action do
request.content_security_policy_nonce_generator ||=
->(_request) { SecureRandom.base64(16) }
request.content_security_policy_nonce_generator ||= ->(_request) { SecureRandom.base64(16) }
request.content_security_policy_nonce_directives = ["style-src"]
end

Expand Down
20 changes: 7 additions & 13 deletions app/models/maintenance_tasks/task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ class NotFoundError < NameError; end
class_attribute :throttle_conditions, default: []

# @api private
class_attribute :collection_builder_strategy,
default: NullCollectionBuilder.new
class_attribute :collection_builder_strategy, default: NullCollectionBuilder.new

define_callbacks :start, :complete, :error, :cancel, :pause, :interrupt

Expand Down Expand Up @@ -64,23 +63,20 @@ def csv_collection(in_batches: nil)
"To resolve this issue run: bin/rails active_storage:install"
end

if in_batches
self.collection_builder_strategy =
BatchCsvCollectionBuilder.new(in_batches)
self.collection_builder_strategy = if in_batches
BatchCsvCollectionBuilder.new(in_batches)
else
self.collection_builder_strategy = CsvCollectionBuilder.new
CsvCollectionBuilder.new
end
end

# Make this a Task that calls #process once, instead of iterating over
# a collection.
def no_collection
self.collection_builder_strategy =
MaintenanceTasks::NoCollectionBuilder.new
self.collection_builder_strategy = MaintenanceTasks::NoCollectionBuilder.new
end

delegate :has_csv_content?, :no_collection?,
to: :collection_builder_strategy
delegate :has_csv_content?, :no_collection?, to: :collection_builder_strategy

# Processes one item.
#
Expand Down Expand Up @@ -122,9 +118,7 @@ def throttle_on(backoff: 30.seconds, &condition)
backoff_as_proc = backoff
backoff_as_proc = -> { backoff } unless backoff.respond_to?(:call)

self.throttle_conditions += [
{ throttle_on: condition, backoff: backoff_as_proc },
]
self.throttle_conditions += [{ throttle_on: condition, backoff: backoff_as_proc }]
end

# Initialize a callback to run after the task starts.
Expand Down
16 changes: 5 additions & 11 deletions test/models/maintenance_tasks/run_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,7 @@ class RunTest < ActiveSupport::TestCase
end

test "#persist_transition calls the complete callback" do
run = Run.create!(task_name: "Maintenance::CallbackTestTask",
status: "running")
run = Run.create!(task_name: "Maintenance::CallbackTestTask", status: "running")
run.status = :succeeded
run.task.expects(:after_complete_callback)
run.persist_transition
Expand All @@ -90,16 +89,14 @@ class RunTest < ActiveSupport::TestCase
end

test "#persist_transition calls the pause callback" do
run = Run.create!(task_name: "Maintenance::CallbackTestTask",
status: "pausing")
run = Run.create!(task_name: "Maintenance::CallbackTestTask", status: "pausing")
run.status = :paused
run.task.expects(:after_pause_callback)
run.persist_transition
end

test "#persist_transition with a race condition moves the run to the proper status and calls the right callback" do
run = Run.create!(task_name: "Maintenance::CallbackTestTask",
status: "running")
run = Run.create!(task_name: "Maintenance::CallbackTestTask", status: "running")
Run.find(run.id).cancelling!

run.task.expects(:after_interrupt_callback).never
Expand All @@ -111,8 +108,7 @@ class RunTest < ActiveSupport::TestCase
end

test "#persist_transition with a race condition for a successful run moves to the succeeded status and calls the right callback" do
run = Run.create!(task_name: "Maintenance::CallbackTestTask",
status: "running")
run = Run.create!(task_name: "Maintenance::CallbackTestTask", status: "running")
Run.find(run.id).cancelling!

run.task.expects(:after_interrupt_callback).never
Expand Down Expand Up @@ -676,9 +672,7 @@ def count_uncached_queries(&block)
query_cb = ->(*, payload) do
count += 1 if !payload[:cached] && payload[:sql] != "SHOW search_path"
end
ActiveSupport::Notifications.subscribed(query_cb,
"sql.active_record",
&block)
ActiveSupport::Notifications.subscribed(query_cb, "sql.active_record", &block)

count
end
Expand Down
24 changes: 7 additions & 17 deletions test/system/maintenance_tasks/runs_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,7 @@ class RunsTest < ApplicationSystemTestCase
fill_in("_task_arguments_post_ids", with: "xyz")
click_on "Run"

alert_text = "Validation failed: Arguments are invalid: :post_ids is "\
"invalid"
assert_text alert_text
assert_text "Validation failed: Arguments are invalid: :post_ids is invalid"
end

test "download the CSV attached to a run for a CSV Task" do
Expand All @@ -72,8 +70,7 @@ class RunsTest < ApplicationSystemTestCase

click_on("Download CSV")

downloaded_csv = "test/dummy/tmp/downloads/"\
"20200109T094144Z_maintenance_import_posts_task.csv"
downloaded_csv = "test/dummy/tmp/downloads/20200109T094144Z_maintenance_import_posts_task.csv"

Timeout.timeout(1) do
sleep(0.1) until File.exist?(downloaded_csv)
Expand Down Expand Up @@ -163,8 +160,7 @@ class RunsTest < ApplicationSystemTestCase
end

assert_text "Errored"
assert_text "Ran for less than 5 seconds until an error happened "\
"less than a minute ago."
assert_text "Ran for less than 5 seconds until an error happened less than a minute ago."
assert_text "ArgumentError"
assert_text "Something went wrong"
assert_text "app/tasks/maintenance/error_task.rb:10:in `process'"
Expand All @@ -190,25 +186,21 @@ class RunsTest < ApplicationSystemTestCase

click_on "Resume"

alert_text = "Validation failed: " \
"Status Cannot transition run from status enqueued to enqueued"
assert_text alert_text
assert_text "Validation failed: Status Cannot transition run from status enqueued to enqueued"
end

test "errors when enqueuing are shown" do
visit maintenance_tasks_path

click_on "Maintenance::EnqueueErrorTask"
click_on "Run"
assert_text "The job to perform Maintenance::EnqueueErrorTask "\
"could not be enqueued"
assert_text "The job to perform Maintenance::EnqueueErrorTask could not be enqueued"
assert_text "Error enqueuing"

visit maintenance_tasks_path
click_on "Maintenance::CancelledEnqueueTask"
click_on "Run"
assert_text "The job to perform Maintenance::CancelledEnqueueTask "\
"could not be enqueued"
assert_text "The job to perform Maintenance::CancelledEnqueueTask could not be enqueued"
assert_text "The job to perform Maintenance::CancelledEnqueueTask "\
"could not be enqueued. Enqueuing has been prevented by a callback."
end
Expand All @@ -227,9 +219,7 @@ class RunsTest < ApplicationSystemTestCase

click_on "Pause"

alert_text = "Validation failed: " \
"Status Cannot transition run from status cancelling to pausing"
assert_text alert_text
assert_text "Validation failed: Status Cannot transition run from status cancelling to pausing"
end
end
end
22 changes: 5 additions & 17 deletions test/validators/maintenance_tasks/run_status_validator_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,7 @@ class RunStatusValidatorTest < ActiveSupport::TestCase

assert pausing_run.valid?

assert_no_invalid_transitions(
[:enqueued, :running, :interrupted, :pausing],
:cancelling
)
assert_no_invalid_transitions([:enqueued, :running, :interrupted, :pausing], :cancelling)
end

test "run can go from enqueued, interrupted or running to pausing" do
Expand All @@ -122,10 +119,7 @@ class RunStatusValidatorTest < ActiveSupport::TestCase

assert running_run.valid?

assert_no_invalid_transitions(
[:enqueued, :interrupted, :running],
:pausing
)
assert_no_invalid_transitions([:enqueued, :interrupted, :running], :pausing)
end

test "run can go from pausing to paused" do
Expand Down Expand Up @@ -210,10 +204,7 @@ class RunStatusValidatorTest < ActiveSupport::TestCase

assert cancelling_run.valid?

assert_no_invalid_transitions(
[:enqueued, :running, :pausing, :interrupted, :cancelling],
:errored
)
assert_no_invalid_transitions([:enqueued, :running, :pausing, :interrupted, :cancelling], :errored)
end

private
Expand All @@ -228,11 +219,8 @@ def assert_no_invalid_transitions(valid_starting_statuses, end_status)

run.status = end_status

refute(run.valid?,
"Expected transition from #{status} to #{end_status} to be invalid")
expected_status_error = [
"Cannot transition run from status #{status} to #{end_status}",
]
refute(run.valid?, "Expected transition from #{status} to #{end_status} to be invalid")
expected_status_error = ["Cannot transition run from status #{status} to #{end_status}"]
assert_equal(expected_status_error, run.errors[:status])
end
end
Expand Down

0 comments on commit 4dfd9d6

Please sign in to comment.