Enable random order for specs #2466
Conversation
|
|
||
| # Follow has an after_create callback that creates a channel between the two users, | ||
| # so to make sure this test is correct, we delete all channels right after | ||
| ChatChannelMembership.delete_all |
There was a problem hiding this comment.
As I specified in the comment, the Follow model has an after_create callback whose side effect is to create a ChatChannel. The job doesn't create a new channel if there's an existing one. I honestly have no idea what was the combination and the sequence of test leading to no channels so that this one would pass but by removing channels explicitly we can ensure the test always passes
There was a problem hiding this comment.
The callback contains the call to the job perform_later, which won't execute if the queue_adapter is set to test, so the channel was not actually created in the callback and the test passed.
However, I agree that the previous version of the test is not clear regarding the ChatChannel creation, so explicitly deleting makes sense.
rhymes
changed the title
A few tests are running by chance because the correct combination of variables has been set before execution. Some of these "race conditions" have been surfaced by running in order. The queue adapter value is one of them. Also using ActiveJob::TestHelper helpers.
There was a problem hiding this comment.
Amazing job. Thanks, @rhymes
I thought about the possible problem with the ActiveJob.queue_adapter inconsistency, but haven't worked on it.
pr-triage
bot
added
PR: reviewed-approved
pr-triage
bot
added
PR: unreviewed
* master: (83 commits) Update gitdocs (forem#2500) Condense 'ask me anything' to 'ama' (forem#2428) [ci skip] Add user_signed_in? to cache key for styles (forem#2498) Added troubleshooting for byebug without readline issue (forem#2481) Fix some frontend linting issues (forem#2495) [ci skip] Fix <br/> in footer. (forem#2491) Remove extra param and add message for prefill (forem#2487) Make Cards Change Dynamically and add Reader/Follower Charts (forem#2488) Temporarily comment out random (forem#2486) Add nav buttons to pwa desktop (forem#2484) Feature/filtered charts (forem#2482) Add caching for historical data (forem#2476) There are installation sections for other OSes now. (forem#2480) Add inbox guidelines to users (forem#2473) Release Open Inbox (forem#2468) Convert underscores in article slugs properly (forem#2472) Update framework defaults to match those in Rails 5.1 (forem#2309) Enable random order for specs (forem#2466) [ci skip] forem#118 Allow users to embed Medium posts with Liquid Tags (forem#1161) Allow API to return top articles (forem#2469) ... # Conflicts: # Envfile # app/models/user.rb # db/schema.rb
What type of PR is this? (check all applicable)
Description
Running specs in a random order it's a great way to see if there are dependency problems among tests or weird race conditions.
The great thing about this is that by providing the seed with the failing test in a subsequent run it's possible to reproduce the conditions that lead to the tests failing.
I had to make some modifications to some tests to increase robustness (and get rid of the weird behaviors on sequencing and active job related tests)
Added to documentation?