Shopify / shipit-engine

Deployment coordination
https://shopify.engineering/introducing-shipit
MIT License
1.42k stars 145 forks source link

Fix intermittent test failures #359

Closed byroot closed 9 years ago

byroot commented 9 years ago

We must have a leaky test or something, I noticed a couple unexplained and intermittent test failures:

  1) Failure:
StatusTest#test_.replicate_from_github!_is_idempotent [/Users/byroot/workspace/shopify/shipit/vendor/shipster/test/models/status_test.rb:14]:
"@commit.statuses.count" didn't change by 0.
Expected: 2
  Actual: 3

and

  1) Failure:
RollbacksControllerTest#test_:create_with_`force`_option_ignore_the_active_deploys [/home/ubuntu/shipit/vendor/shipster/test/controllers/rollbacks_controller_test.rb:37]:
"@stack.deploys.count" didn't change by 1.
Expected: 6
  Actual: 5
gmalette commented 9 years ago

Do you have the seed?

byroot commented 9 years ago

No I don't for some reason it's not printed out. That would be the first thing to fix.

byroot commented 9 years ago

Nevermind: Run options: --seed 8804

byroot commented 9 years ago

Ok, so it appear that it isn't order dependent (I retried previously failed builds with the same seed and they passed).

So this feels like a huge rabbit hole I'll let on the side for now.

gmalette commented 9 years ago

So this seems to be due to the fact that Rollbacks are loaded the Deploys count, but only AFTER Rollbacks have been loaded in the first place (obv).

> s = Stack.first
> s.deploys.count
#   (3.0ms)  SELECT COUNT(*) FROM `tasks` WHERE `tasks`.`type` IN ('Deploy') AND `tasks`.`stack_id` 
> s.rollbacks
#  Rollback Load (0.7ms)  SELECT `tasks`.* FROM `tasks` WHERE `tasks`.`type` IN ('Rollback') AND `tasks`.`stack_id` = 3
> s.deploys.count
#   (0.9ms)  SELECT COUNT(*) FROM `tasks` WHERE `tasks`.`type` IN ('Deploy') AND `tasks`.`stack_id`
> s.deploys(true).count
#  Deploy Load (0.8ms)  SELECT `tasks`.* FROM `tasks` WHERE `tasks`.`type` IN ('Deploy', 'Rollback') AND `tasks`.`stack_id` = 3
#   (0.5ms)  SELECT COUNT(*) FROM `tasks` WHERE `tasks`.`type` IN ('Deploy', 'Rollback') AND `tasks`.`stack_id` = 3
gmalette commented 9 years ago

Obvious fix for tests is to eagerload models, but this means that dev could have discrepancies

byroot commented 9 years ago

Nice catch. require_dependency should fix it no?

gmalette commented 9 years ago

In Stack? yes