PR #33995 added support for specifying the `args` argument of
`assert_enqueued_with` and `assert_performed_with` as a matcher proc.
In doing so, it added undocumented support for specifying the other
arguments as matcher procs as well. This commit officially documents
that support, and adds tests to ensure the behavior.
The example tests in the method docs for `assert_enqueued_with` and
`assert_performed_with` previously specified `queue` args in their
assertions but not in their setups. Thus, the example tests would not
pass as written. This commit fixes the examples, and properly
demonstrates the `queue` arg.
assert_enqueued_with with a block ignores all the jobs enqueued before
the block for its assertions by counting the number of jobs and dropping
the n first elements from the Array, but since we're now mutating the
Array in perform_enqueued_jobs without a block, it's broken.
This uses another implementation which is correct when the array is
mutated, by getting a duplicated array of jobs, then removing them from
the original array.
Similarly assert_enqueued_jobs with a block was using counts only, now
keeps track of the specific jobs to count them at the end.
Example failure: https://buildkite.com/rails/rails/builds/68010#749ce70d-1e01-4419-90e5-ee4531f66466/1057-1069
Support for testing jobs with a relative time delay was added in #36767.
It was implemented by truncating the `usec` portion of the `at` time in
order to allow for sub-second time differences. However, sub-second
time differences can occur across seconds. Thus, this commit changes
`at` time matching to use an explicit time range.
I guess `NO` is pretty self-explanatory here. But, to be consistent, this commit
describes what does **NO** mean in the context of: `retries, timeaout and priorities`.
This makes sure jobs don't run twice if `perform_enqueued_jobs` is
called twice without a block.
This also mimics the behavior of using `perform_enqueued_jobs` with a
block, where at the end of the block performed jobs are not in
`enqueued_jobs` but instead in `performed_jobs`.
- ### Problem
If we use `perform_enqueued_jobs` without a block, a job that
uses a retry mechanism to reeenqueue itself would get performed
right away.
This behaviour make sense when using `perform_enqueued_jobs` with
a block.
However I'm expecting `perform_enqueued_jobs` without a block to
perform jobs that are **already** in the queue not the ones that
will get enqueued afterwards.
### Solution
Dup the array of jobs given to avoid future mutation.
- ### Problem
If we use `perform_enqueued_jobs` without a block,
a job that raises an error wouldn't be appended to
the list of `performed_jobs`.
### Solution
Push the job in the array before it is actually performed.
author Aditya Narsapurkar <adityanarsapurkar@yahoo.com> 1582316102 +0530
committer Aditya Narsapurkar <adityanarsapurkar@yahoo.com> 1583159505 +0530
parent 6d0895a4894724e1a923a514daad8fb3c9ac2c28
author Aditya Narsapurkar <adityanarsapurkar@yahoo.com> 1582316102 +0530
committer Aditya Narsapurkar <adityanarsapurkar@yahoo.com> 1583159327 +0530
Randomize jitter
- This PR attempts to fix a problem with ActiveJob jitter where the `determine_jitter_for_delay` value may not always be randomized. Especially when the jitter delay multplier is between 1 and 2 it always returns 0.
- With this change, we pass a range to `Kernel.rand` beginning with 0 to the `jitter_multiplier`. With positive float values, the return value will be a random float number from the range.
- Includes test cases to verify random wait time when the jitter_multiplier is between 1 and 2.
- Updated relevant test cases stubbing the `Kernel.rand` method, refactored some by removing unwanted stubs for `Kernel.rand` method where jitter is falsey.
Fixed rubocop issue - used assert_not_equal instead of refute_equal in test case
Fixed rubocop issue - used assert_not_equal instead of refute_equal in test case
Fixed rubocop issue - used assert_not_equal instead of refute_equal in test case
Review updates - separated test cases for random wait time with default and exponentially retrying jobs
- Another test case added to make sure negative wait time does not affect the randomization
Review updates
- Instead of using Kernel.rand with range, used simple multiplication with Kernel.rand for calculating delay for jitter
- Updates to the tests according to changes
- ### Problem
In some cirumstances, the deprecation message to warn that AJ won't
run `after_(enqueue/perform)` callbacks when the chain is halted
by a `throw(:abort)` will be thrown even though no `throw(:abort)`
was thrown.
```ruby
run_callback(:foo) do
...
end
```
There is two possible way for the callback body to not be executed:
1) `before` callback throw a `abort`
2) `before` callback raises an error which is rescued by an
around callback (See associated test in this commit for
an example)
When 2) happen we don't want to output a deprecation message,
because what the message says isn't true and doesn't apply.
### Solution
In order to differentiate between 1) and 2), I have added
a `halted_callback_hook` which is called by ActiveSupport callback
whenever the callback chain is halted.
- The `halted_callback_hook` method is called whenever the
`terminator` halt the callback execution.
Usually, this translate to when a `before` callback throw
an `:abort`.
<details>
<summary> Example </summary>
```ruby
class Foo
include ActiveSupport::Callbacks
define_callbacks :save
set_callback(:save, :before) { throw(:abort) }
def run
run_callbacks(:save) do
'hello'
end
end
def halted_callback_hook(filter)
# filter is the proc passed to `set_callback` above
end
end
```
</details>
### Problem
When a class has multiple callbacks, (i.e. `save`, `validate` ...),
it's impossible to tell in the halted_callback_hook which type of
callback halted the execution.
This is useful to take different action based on the callback.
<details>
<summary> Use Case </summary>
```ruby
class Foo
include ActiveSupport::Callbacks
define_callbacks :save
define_callbacks :validate
set_callback(:save, :before) { throw(:abort) }
set_callback(:validate, :before) { throw(:abort) }
def run
run_callbacks(:validate) do
...
end
run_callbacks(:save) do
...
end
end
def halted_callback_hook(filter)
Rails.logger.warn("Couldn't save the record, the ??? callback halted the execution")
end
end
```
</details>
### Solution
Allow `halted_callback_hook` to receive a second argument which is
the name of the callback being run.
- ### Problem
Given the below example the test adapter will retry the job
indefinitely:
```ruby
class BuggyJob < ActiveJob::Base
retry_on(Exception, attempts: 2)
def perform
raise "error"
end
end
BuggyJob.perform_later
perform_enqueued_jobs
```
The problem is that when the job get retried, the
`exception_executions` variable is not serialized/deserialized,
resulting in ActiveJob to not be able to determine how many time
this job was retried.
The solution in this PR is to deserialize the whole job in the test
adapter, and reserialize it before retrying.
Fix#38391
- I made a change in 0d3aec49695 to output a log if a job was aborted
in a before callbacks. I didn't take in consideration that a job
could return a falsy value and thus it would wrongly log
that the job was aborted.
This fixes the problem by checking if the callback chain was halted
rather than the return value of the job.
- Since this is going to be the default in 6.1, let's set it in the
test suite to avoid deprecation warning.
Otherwise one has to do `AS::Deprecation.silence { }` everytime we
add a new test.
Fix#38107
* Add failing ActiveJob exceptions test for "disable retry jitter"
Thanks to @kaspth for the starting point.
* Update ActiveJob retry jitter to correctly use zero value
* Simplify "disable retry jitter" test
We don't need to repeat this many times. Fewer is shorter.
* Refactor determine_delay with jitter
* Fix indentation
* Close the curtains and give JITTER_DEFAULT some privacy
* Use .zero? instead of == to check jitter value
* Add ActiveJob test for explicit zero jitter
Co-authored-by: Kasper Timm Hansen <hey@kaspth.com>
Co-authored-by: Cliff Pruitt <cliff.pruitt@cliffpruitt.com>
ActiveJob will log the entire backtrace when one of the enqueue
callbacks fail. This is not necessary and increases noise. When there is
an Exception object, the object gets bubbled up eventually anyways.
Remove `Array(ex.backtrace).join("\n")` from `def enqueue` and `def
enqueue_at`.
The existing message only mentioned one type of before/after callback,
but the config was named generally. That mismatch is confusing and users
wouldn't necessarily know what the total effect of the config would be.
So instead of handwriting the deprecation warning in the specific instances,
consolidate it in one place and give the appropriate context. That context
is the above, but also that users shouldn't update their app config,
they should uncomment the line in the new defaults file, which now also
has more context.
I'm not totally convinced that we can't move this to when
`after_enqueue`/`after_perform` is called in the job class. Doesn't
seem worth it to blare this after every job enqueue/perform, when we
the score at boot time.
cc @Edouard-chin
-
### Problem
In rails/rails@bbfab0b33a I introduced a change which outputs
a deprecation whenever a class inherits from ActiveJob::Base.
This has the negative effect to trigger a massive amount of
deprecation at boot time especially if your app is eagerloaded
(like it's usually the case on CI).
Another issue with this approach was that the deprecation will
be output no matter if a job define a `after_perform` callbacks
i.e.
```ruby
class MyJob < AJ::Base
before_enqueue { throw(:abort) }
end
# This shouldn't trigger a deprecation since no after callbacks are defined
# The change in 6.2 will be already safe for the app.
```
### Solution
Trigger the deprecation only when a job is abort
(during enqueuing or performing) AND a `after_perform`
callback is defined on the job.
