Fixes for failing test cases #852
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…priority to guarantee priority is respected among jobs scheduled for the same date
…times the test fails due to an imprecission of a milisecond which is an acceptable imprecission
…ven though we do not delete jobs on agend.stop()
koresar
reviewed
Sep 10, 2019
dmbarreiro
commented
Sep 11, 2019
|
|
||
| agenda._collection.findOne({name: 'longRunningJob'}, (err, job) => { |
There was a problem hiding this comment.
I suspect there was an await missing here, that caused the afterEach mocha section of the test to be executed clearing the jobs which eventually made the findOne return undefined, no job found.
koresar
reviewed
Sep 11, 2019
…ault behavior in mocha
Closed
xenokratos
pushed a commit
to beedeez/agenda
that referenced
this pull request
Mar 23, 2021
These modifications try to fix an issue with failing test cases which are preventing contributions to make it to the repo (when these contributions have nothing to do with the tests themselves). The changes are: * new queuing mechanism since the previous didn't guarantee priority when executing jobs scheduled the same datetime (agenda uses setTiemout and sometimes the job with lowest priority was executed, though not processed, first). Test case involved: job concurrency -> 'should run jobs as first in first out (FIFO) with respect to priority' * On repeatEvery -> 'sets the nextRunAt property with skipImmediate' test case we expect precise timing in milliseconds and that's not always the with the processing we do to set repeatEvery, sometimes we schedule the job one millisecond after the expected time, I find this acceptable and changed the test case to expect something in the range of 3 minutes sharp and 3 minutes + 2 milliseconds for nextRunAt. * start/stop -> 'clears locks on stop' fails on many PRs not being able to find 'longRunningJob' on db. I provided the param job on define callback since not having params has caused trouble before and increased jobTimeout when we execute the tests on TravisCI (actually I only set the TRAVIS env var on .travis.yml, the tests were already prepared to increase this timeout when that env var is present). Hopefully it'll help, never suffered the issue when testing locally, I've only seen it on TravisCI.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These modifications try to fix an issue with failing test cases which are preventing contributions to make it to the repo (when these contributinos have nothing to do with the tests themselves).
The changes are:
new queuing mechanism since the previous didn't guarantee priority when executing jobs scheduled the same datetime (agenda uses setTiemout and sometimes the job with lowest priority was executed, though not processed, first). Test case involved: job concurrency -> 'should run jobs as first in first out (FIFO) with respect to priority'
On repeatEvery -> 'sets the nextRunAt property with skipImmediate' test case we expect precise timing in miliseconds and that's not always the with the processing we do to set repeatEvery, sometimes we schedule the job one milisecond after the expected time, I find this acceptable and changed the test case to expect something in the range of 3 minutes sharp and 3 minutes + 2 miliseconds for nextRunAt.
start/stop -> 'clears locks on stop' fails on many PRs not being able to find 'longRunningJob' on db. I provided the param job on define callback since not having params has caused trouble before and increased jobTimeout when we execute the tests on TravisCI (actually I only set the TRAVIS env var on .travis.yml, the tests were already prepared to increase this timeout when that env var is present). Hopefully it'll help, never suffered the issue when testing locally, I've only seen it on TravisCI.