Removes rackspace-specific logic from identity

[elight]

Providers should not be registered in fog-core. Period.

For Fog 2.0, we need to ensure all providers namespace the same way. We shouldn't have to do something like what was done in this PR...

[elight]

So @wchrisjohnson and @krames both prefer matchers. I prefer xUnit-style assertions and used those when I ported the shindo tests for fog-core to minitest.

But this is bigger than this PR and our 3 opinions.

/cc @icco @geemus @tokengeek @mwhagedorn @nirvdrum

[wchrisjohnson]

@elight So how do we handle the current scenario where we have a versioned Identity class in play? We need to differentiate between versions of Identity. We v1, v2 and v3 of the Identity service, based on the refactor I am currently working on.

Are we forcing the usage of files named like identity_v2.rb and class names like IdentityV2? Yuck (IMO). Do you see a way (in fog-core) we can accommodate a versioned Identity class? This will be a pervasive need across services sooner rather than later...

As a reference, I have a hacked version of the Fog::Identity.new method doing this:

require "fog/openstackcommon/identity_v2"
return Fog::Identity::V2::OpenStackCommon.new(attributes)

I may be overlooking something, and would be thrilled if that ends up being the case.

[nirvdrum]

Despite running a testing company (am I the only collaborator that doesn't work for a service provider? :-P), I find most testing discussions to be a waste of time. They're usually not driven by anything that resembles data and are often quickly side-tracked by things that don't matter (ever argue whether something was an integration test or a functional test?)

I happen to think a method named test_storage_provider_stores_file_without_error sucks, but so does:

describe 'storage provider' do
  describe '#store_file' do
    context 'when successful' do
      it 'should store without error' do
      end
    end
  end
end

Which is to say, we have a lot going on and both approaches have trade-offs. For my part, I tend to think better in terms of assertions and find a lot of the BDD syntax to be ceremonious. I can buy the argument that the latter reads more like a spec. It's ruby -- I can work with either. At this point, I'm happy to do whatever isn't shindo (sorry @geemus!). And I'll take working code over debate, so race for it :-)

[tokengeek]

@elight - Emails arrived in jumbled order. Had commented on the line before I got to the PR itself!

Basically since there were so many opinions it came down to @geemus preference, which was Test::Unit style minitest.

After a bit of searching I found the original... fog/fog#1250 (comment) was from the Roadmap thread not the later testing one but it stuck in my mind.

My "preference" was RSpec due to familiarity (and availability of tagging) so please don't think I'm grumbling because I didn't get my own way! 😄 I wasn't even that precious about RSpec.

The amount of tests I need to rewrite in fog and fog-json are not epic so I don't mind changing if @geemus is happy with that.

I know we mentioned the possibility about each provider doing their own tests their own way but we never really resolved a way to run "integration" tests across everyone's code.

Yes we can update the rake task to run both spec and test but that feels disjointed.

[wchrisjohnson]

Looks good to me - would appreciate this getting merged as soon as practical.

[wchrisjohnson]

I believe this line should be self.providers.include?(provider) or Fog.providers.include?(provider).

As-is, it's breaking for me in my hacked up version.

[elight]

Odd. It's implicitly a call to Fog::Identity::providers as the call is occurring in a Fog::Identity::new.

[wchrisjohnson]

My bad - have verified that this DOES work - apologies.

[geemus]

I'm open to discussing how we structure tests. I definitely prefer assert_stuff(arguments) for the actual tests, compared to monkey-patch rspec style matchers. In terms of building context long_method_names seem to get clunky pretty quickly and don't allow nesting, which often can help keep things clean, so the it/should spec style for those is maybe preferred? I feel like that might fit somewhere in the middle of the proposed solutions above, for better or worse...

[elight]

That sounds like the style that I used in my tests: MiniTest Spec structure with MiniTest xUnit assertions.

[tokengeek]

@geemus As long as we can decide on something soon!

I've got tests to rewrite anyway across two or three projects and more changes that are getting held up because I've been waiting for a decision.

[geemus]

@elight @tokengeek - yep sounds good. I say we stick with the spec structure/unit assertions. I think that gives a kind of best-of-both-worlds.

[elight]

Everyone else good with that?

[krames]

@elight Looks like travis is failing. I think @tokengeek fixed this issue in master. Can you rebase?

[tokengeek]

Yep. Okay with the testing decision. It'll be a while whilst I transition stuff back and forth.

And yep, the Travis error (Rake dropping 1.8.7) support has been fixed

[tokengeek]

Unless the decision is an April's Fool joke in which case it's not funny.

[geemus]

@tokengeek hahaha. Poor timing, but no I mean that wholly, go for it!

[elight]

That would have to be the lamest April Fool’s joke I’ve ever seen. ;-)

On April 1, 2014 at 10:53:23 AM, Paul Thornthwaite (notifications@github.com) wrote:

Unless the decision is an April's Fool joke in which case it's not funny.

—
Reply to this email directly or view it on GitHub.

[geemus]

Thanks!

On Mon, Apr 7, 2014 at 11:54 AM, Evan Light notifications@github.comwrote:

Merged #30 #30.

Reply to this email directly or view it on GitHubhttps://github.com//pull/30
.