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

use `descendants` to get class that inherited `ActiveJob::Base` #26391

Merged
merged 1 commit into from Sep 5, 2016

Conversation

Projects
None yet
6 participants
@y-yagi
Member

y-yagi commented Sep 5, 2016

Summary

subclasses get only child classes.
Therefore, if create a job common parent class as ApplicationJob,
inherited class does not get properly.

use `descendants` to get class that inherited `ActiveJob::Base`
`subclasses` get only child classes.
Therefore, if create a job common parent class as `ApplicationJob`,
inherited class does not get properly.
@rails-bot

This comment has been minimized.

rails-bot commented Sep 5, 2016

r? @eileencodes

(@rails-bot has picked a reviewer for you, use r? to override)

class InheritedJobTest < ActiveJob::TestCase
def test_queue_adapter_is_test_adapter
assert_instance_of ActiveJob::QueueAdapters::TestAdapter, InheritedJob.queue_adapter

This comment has been minimized.

@pixeltrix

pixeltrix Sep 5, 2016

Member

Shouldn't this be an instance of InlineAdapter? That's what it's been set to:

class InheritedJob < ApplicationJob
  self.queue_adapter = :inline
end

This comment has been minimized.

@prathamesh-sonpatki

prathamesh-sonpatki Sep 5, 2016

Member

We reset the inline adapter to test adapter here

klass.queue_adapter = test_adapter

This comment has been minimized.

@pixeltrix

pixeltrix Sep 5, 2016

Member

Ahh, understand now - it's testing that the adapter is overridden. Sorry - looks good 👍

@pixeltrix pixeltrix merged commit 1e282df into rails:master Sep 5, 2016

2 checks passed

codeclimate Code Climate didn't find any new or fixed issues.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@pixeltrix

This comment has been minimized.

Member

pixeltrix commented Sep 5, 2016

Backported in 74a49ba

@pixeltrix

This comment has been minimized.

Member

pixeltrix commented Sep 5, 2016

Is this something that needs fixing in 4.2 ?

@y-yagi

This comment has been minimized.

Member

y-yagi commented Sep 5, 2016

Is this something that needs fixing in 4.2 ?

No. Set of adapter for each class was added in 5.0.0( #16992 ) . Therefore, there is no effect in 4.2.

@y-yagi y-yagi deleted the y-yagi:use_descendants_to_get_all_subclasses branch Sep 5, 2016

@pixeltrix

This comment has been minimized.

Member

pixeltrix commented Sep 6, 2016

@y-yagi thanks for the the clarification ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment