Skip to content

Conversation

@whilefalse
Copy link
Contributor

Hi,

Thanks for the awesome plugin, has been really useful. I noticed it was lacking a little in tests so I've brought it up to 100% code coverage.

Note that I've also changes to the 1.9 version of xmlcanonicalizer, so I'm not sure if you'll be able to merge this in - feel free to pick just the specs and not the gemspec though.

Thanks,
Steve

…xmlcanonicalizer which works (I was getting weird issues with the other one)
100% test coverage
@rajiv
Copy link
Contributor

rajiv commented Mar 27, 2012

thanks for the updates. running locally on ruby 1.9.2p180, i'm seeing these test failures:

Failures:

  1) OmniAuth::Strategies::SAML::AuthResponse valid? it should behave like a validating method when the current time is between the NotBefore and NotOnOrAfter times 
     Failure/Error: it { should be_valid }
       expected valid? to return true, got false
     Shared Example Group: "a validating method" called from ./spec/omniauth/strategies/saml/auth_response_spec.rb:84
     # ./spec/shared/validating_method.rb:3:in `block in assert_is_valid'

  2) OmniAuth::Strategies::SAML::AuthResponse validate! it should behave like a validating method when the current time is between the NotBefore and NotOnOrAfter times should be valid
     Failure/Error: expect { subject.validate! }.not_to raise_error
       expected no Exception, got #<OmniAuth::Strategies::SAML::ValidationError: Current time is on or after NotOnOrAfter condition>
     Shared Example Group: "a validating method" called from ./spec/omniauth/strategies/saml/auth_response_spec.rb:88
     # ./spec/shared/validating_method.rb:6:in `block in assert_is_valid'

  3) OmniAuth::Strategies::SAML POST /auth/saml/callback when the response is valid should set the uid to the nameID in the SAML response
     Failure/Error: auth_hash['uid'].should == 'THISISANAMEID'
     NoMethodError:
       undefined method `[]' for nil:NilClass
     # ./spec/omniauth/strategies/saml_spec.rb:53:in `block (4 levels) in <top (required)>'

  4) OmniAuth::Strategies::SAML POST /auth/saml/callback when the response is valid should set the raw info to all attributes
     Failure/Error: auth_hash['extra']['raw_info'].to_hash.should == {
     NoMethodError:
       undefined method `[]' for nil:NilClass
     # ./spec/omniauth/strategies/saml_spec.rb:57:in `block (4 levels) in <top (required)>'

@whilefalse
Copy link
Contributor Author

Hmm, interesting.

I think I'm running ruby 1.9.3 so I don't know if that could be a problem. I will check it out.

@rajiv
Copy link
Contributor

rajiv commented Mar 27, 2012

thanks. its fine to require 1.9.2. however our application is not yet running on 1.9.3.

@whilefalse
Copy link
Contributor Author

Hey,

I think I narrowed it down to a time zone issue in my tests in the end - that's the problem with the UK - it's always GMT so we forget about time zones!

Anyway, I think they should pass for you now.

Cheers,
Steve

@rajiv
Copy link
Contributor

rajiv commented Mar 30, 2012

squashed and merged in 0f5ec78 and f9a388f. thanks!

@rajiv rajiv closed this Mar 30, 2012
rajiv pushed a commit that referenced this pull request Dec 4, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants