Skip to content

Conversation

@gerardo-navarro
Copy link
Contributor

Summary

  • move logout response and request specs into a dedicated POST /auth/saml/slo describe block
  • move SP-initiated SLO specs into their own POST /auth/saml/spslo describe block
  • leave the callback describe focused on callback-specific scenarios

Testing

bundle exec rspec spec/omniauth/strategies/saml_spec.rb

Copy link
Contributor Author

@gerardo-navarro gerardo-navarro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bufferoverflow Hi 👋 Can you please have a look at this small refactoring that makes the tests more readable.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the test organization in saml_spec.rb by extracting nested contexts for logout-related endpoints into separate top-level describe blocks, improving test structure and readability.

  • Extracts /auth/saml/slo tests into a dedicated describe block
  • Extracts /auth/saml/spslo tests into a dedicated describe block
  • Consolidates shared setup (sp_entity_id) at appropriate scope levels

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@fh1ch fh1ch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gerardo-navarro much appreciated, thanks a ton for the refactoring 🙇

LGTM 👍

@fh1ch fh1ch merged commit f82193a into omniauth:master Nov 3, 2025
10 checks passed
@gerardo-navarro gerardo-navarro deleted the gerardo-navarro-refactor-saml-specs-for-logout-handling branch November 8, 2025 20:09
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.

3 participants