Eliminate global state leaks completely in ActiveModel tests #14342

Merged
merged 1 commit into from Mar 11, 2014

Conversation

Projects
None yet
3 participants
Contributor

zuhao commented Mar 10, 2014

I've gone through every test case in ActiveModel, and made sure there is no global state leaked. Now ActiveModel tests can truly run in random order.

Owner

jeremy commented Mar 10, 2014

Nice review and fixes - thank you!

Need to adjust begin...ensure scope in some tests, squash to a single commit, then we're good to merge.

Completely remove potential global state leaks in ActiveModel tests.
ActiveModel tests can now be run in random order.
Contributor

zuhao commented Mar 10, 2014

@jeremy Thanks. All begin...ensure scopes are adjusted, and commits squashed into a single one.

begin
+ original_bcrypt_cost = BCrypt::Engine.cost
@arthurnn

arthurnn Mar 10, 2014

Member

you could, one line this to:

original_bcrypt_cost, BCrypt::Engine.cost = BCrypt::Engine.cost, 5
Member

arthurnn commented Mar 10, 2014

LGTM ... good job. 👍

jeremy added a commit that referenced this pull request Mar 11, 2014

Merge pull request #14342 from zuhao/eliminate_global_state_leak_in_a…
…ctivemodel_tests

Eliminate global state leaks completely in ActiveModel tests

@jeremy jeremy merged commit 47ea87b into rails:master Mar 11, 2014

1 check passed

default The Travis CI build passed
Details

@zuhao zuhao deleted the zuhao:eliminate_global_state_leak_in_activemodel_tests branch May 11, 2014

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