PeterBorah / vigor

An unofficial ruby wrapper for the League of Legends API
MIT License
4 stars 4 forks source link

Fixes #13 #20

Closed imogenkinsman closed 10 years ago

imogenkinsman commented 10 years ago

I added an error to raise when someone tries to use an unconfigured client.

I also refactored tests a bit - moved error testing to its own spec file, etc. One important consequence of https://github.com/PeterBorah/vigor/commit/5d49f785d236887b0a659c5323fb4276a4ad3f21 is that Vigor is effectively a singleton now. That means that all tests are referring to the same Vigor object, so configuration carries over from test to test - not ideal for testing. I set a before hook in spec_helper.rb to try to isolate tests better:

  c.before(:each) do
    Vigor::Client.default_options = {}
  end
PeterBorah commented 10 years ago

Good stuff, thanks!

One important consequence of 5d49f78 is that Vigor is effectively a singleton now. That means that all tests are referring to the same Vigor object, so configuration carries over from test to test - not ideal for testing.

This is actually exactly why the change was necessary: It's always been that way, it just wasn't obvious before. Initializing a new Vigor::Client used to set various options on the class itself, which would carry over to other Client objects. This seems to be a limitation of httparty: there's no way to easily make distinct instantiations of it.

imogenkinsman commented 10 years ago

This is actually exactly why the change was necessary: It's always been that way, it just wasn't obvious before.

Ah yeah, since base_uri etc are class variables, every time you create a new object you'd affect every other Vigor object.