Skip to content
This repository has been archived by the owner on Oct 13, 2021. It is now read-only.

fix missing parameter. Test cover of Pgbackups class #55

Merged
merged 1 commit into from
Mar 5, 2015
Merged

fix missing parameter. Test cover of Pgbackups class #55

merged 1 commit into from
Mar 5, 2015

Conversation

scruti
Copy link
Contributor

@scruti scruti commented Mar 4, 2015

Here I am again.
After applying changes done at #54, we tried again and found a new problem.

captura de pantalla 2015-03-04 a las 13 50 44

Seems that during a method extraction refactor (f27c084) the typical "oh, I forget to pass the parameter to the new extracted method" (happens too many times to me U_U') was introduced.

So the new method is calling heroku_app_name, variable that is not defined in his scope.

I tried to avoid future errors through a basic test covering of the class. I'm not proud of tests that I created here, since I had to stub A LOT just to be able to test what calls we want to be done.

Also it was very difficult for me trying to isolate expectations to one per test, because if I isolate expectations I would have to create new stubs for the different internal calls at each test. So, tired of trying too complicated things, I decided to move to a "well, it works" approach. Better than nothing.

def existing_backups?
@heroku.client.addon.list(heroku_app_name).select do |addon|
def existing_backups?(heroku_app_name)
@heroku.client.addon.list(heroku_app_name).any? do |addon|
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that any? replace the select then the any? on selected stuff?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it does.
Here you can see another place where any? is being used in same way (and also checking if any addon of the list has a specific name).

2.1.2 :001 > [1,2,3,4,5].select { |num|  num.even?  }
 => [2, 4]
2.1.2 :002 > [1,2,3,4,5].select { |num|  num.even?  }.any?
 => true
2.1.2 :003 > [1,2,3,4,5].any? { |num|  num.even?  }
 => true

@jipiboily
Copy link
Contributor

Other than that question, this looks good to me.

jipiboily added a commit that referenced this pull request Mar 5, 2015
fix missing parameter. Test cover of Pgbackups class
@jipiboily jipiboily merged commit 9ceaee2 into rainforestapp:master Mar 5, 2015
@jipiboily
Copy link
Contributor

New version released on Rubygems. Thanks! :)

@scruti
Copy link
Contributor Author

scruti commented Mar 5, 2015

🆒 !

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants