Delete instances of Parrot after each test. #14332

Merged
merged 1 commit into from Mar 10, 2014

2 participants

@tgxworld

Given that Parrot instances has been created
When the test has been run
Then all instances of Parrot should be deleted

@tgxworld

@senny For your review. Thanks! I'll squash the commits once the review is done :)

@tgxworld tgxworld and 1 other commented on an outdated diff Mar 9, 2014
activerecord/test/cases/autosave_association_test.rb
@ship.delete
@pirate.delete
+ super
@tgxworld
tgxworld added a note Mar 9, 2014

I noticed that we were overriding the method teardown in ActiveRecord::TestCase where SQLCounter.clear_log is called.

@senny
Ruby on Rails member
senny added a note Mar 9, 2014

probably best to use the teardown helper method like so:

teardown do
# ...
end

This won't overwrite the normal teardown method and we can't forget to call super.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@senny senny and 2 others commented on an outdated diff Mar 9, 2014
activerecord/test/cases/autosave_association_test.rb
@@ -35,7 +35,7 @@ def should_be_cool
end
end
}
- reference = Class.new(ActiveRecord::Base) {
+ reference = Class.new(base) {
@senny
Ruby on Rails member
senny added a note Mar 9, 2014

If base is only ever used in these two calls I say we inline ActiveRecord::Base and get rid of the base method. Are there other uses?

reference = Class.new(ActiveRecord::Base) {

  • reference = Class.new(base) {

if u add ur ref will adding in the app.

@tgxworld
tgxworld added a note Mar 9, 2014

@senny Nope the only use cases are here. I was considering between using base or completely remove it but went with the former since the presence of the method signals the intention for DRYing up the code to me.

Hi @sprodhan01 what do you mean?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@senny
Ruby on Rails member

Please do squash the commits. You can always make further review changes using git commit --amend and git push -f afterwards.

@senny senny self-assigned this Mar 9, 2014
@tgxworld

Ok no problem. I wanted to show the commits first so that it will show the reviewer what I'm doing before squashing them.

@tgxworld

@senny Updated :)

@senny
Ruby on Rails member

awesome! Thanks @tgxworld for your contribution 💛

@senny senny merged commit ed9a69a into rails:master Mar 10, 2014

1 check passed

Details default The Travis CI build passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment