Adjust test setup to use OneLogin::RubySaml::Attributes

[doconnor-clintel]

Split from #245

Here I changed the test setup so that we'd get the expected results, rather than relying on global state from the saml gem and changing the test results.

[adamstegman]

Thanks for updating this test! I have one small suggestion to keep it in line with how it is today.

[adamstegman]

After #to_h, this delete won't have mutated the attributes. Since there isn't a great way to delete an attribute after constructing the Attributes object, what if instead we delayed it until after this mutation?

Suggested change

	saml_attributes[saml_key] = saml_attributes.to_h.delete(resource_key)
	saml_attributes[saml_key] = saml_attributes.delete(resource_key)

let(:instance) { described_class.new(OneLogin::RubySaml::Attributes.new(saml_attributes), attribute_map) }
# ...
let(:saml_attributes) do
  {
    # ...
  }
end

[doconnor-clintel]

Other question, does it need to .delete and reassign at all?

When I look at it, I'm not really sure why we're it was originally doing basically:

x = {'foo' => 'bar' }
x.delete 'foo'
x['foo]' = 'bar'

Okay the OneLogin::RubySaml::Attributes acts kind of like a hash, just without all of the methods and interface; but what was the original test proving with the modification?

(When I did this change, it was basically 'see test failure, do thing fix test failure'; not really understanding why)

[adamstegman]

It's hard to say for sure. I feel like it's testing that the original name isn't being used, instead the configured name is being used. Another way of accomplishing this would be to create a new attributes object each time, instead of mutating the shared object.

[doconnor-clintel]

Ah, oops, missed that was was saml_key vs resource_key

[doconnor-clintel]

Unfortunately, I've not had the spare capacity to revisit this one and trial the change.
Closing for the moment.