search

railslove / fontello_rails_converter

CLI gem for comfortably working with icon fonts (open, download, convert) from http://fontello.com for usage in Rails apps.
MIT License
73 stars 35 forks source link

Respect --config-file when downloading #37

Closed Sinetheta closed 8 years ago

Sinetheta commented 8 years ago

We have an option for specifying a config file, but that option is only used when opening a session. Whenever a new build is downloaded we throw the config.json in the same tired old location.

jhilden commented 8 years ago

Thanks a lot for the fix @Sinetheta. Could you maybe provide some testcoverage for the fix you did?

Sinetheta commented 8 years ago

That's a tough one. Since not much has been abstracted here I would have to actually hit (or mock) the file system. Which is generally not such a good thing.

A test for copy_config_json would basically be

  1. Add a config file fixture
  2. Add a .zip fixture
  3. Open the zipfile
  4. Pass them both to copy_config_json
  5. Verify that the contents of the zipfile were placed where the config file specified.
  6. Cleanup any leftover files.

Meaning that the spec would do about 5 times as much work as the method, which is why I imagine that it was never tested in the first place.

Sinetheta commented 8 years ago

I've added a spec that just uses a mock to verify the call. A bit tautological, but hey, it's a test :wink:

jhilden commented 8 years ago

thanks a lot. it might be a only a mock spec, but it will prevent a regression of this particular bug.