-
Notifications
You must be signed in to change notification settings - Fork 0
Cleaning up batch tests #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: batch-poc
Are you sure you want to change the base?
Conversation
| ApplicationJob.enqueue_after_transaction_commit = false if defined?(ApplicationJob.enqueue_after_transaction_commit) | ||
| SolidQueue.preserve_finished_jobs = true | ||
| SolidQueue::Batch.maintenance_queue_name = nil | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these removals in setup and teardown are happening because this is already being handled either in test_helper.rb or by the use of transactional tests
| JobBuffer.add "Hi failure #{batch.id}!" | ||
| end | ||
| end | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just moving those jobs from the bottom to the top of the file
| def job!(active_job) | ||
| SolidQueue::Job.find_by!(active_job_id: active_job.job_id) | ||
| skip_active_record_query_cache do | ||
| SolidQueue::Job.find_by!(active_job_id: active_job.job_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be fine but I just wanted to make sure we are skipping active record query cache, as we do in other tests
|
|
||
| class SolidQueue::BatchTest < ActiveSupport::TestCase | ||
| self.use_transactional_tests = false | ||
| class BatchCompletionJob < ApplicationJob |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved BatchCompletionJob here instead of a separate file, since it's only being used in this test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wasn't being used anywhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved to test/models/solid_queue/batch_test.rb
Following up our discussion at rails#142
These are some minimal improvements to the organization of the tests. Very great work with the batches @jpcamara !