Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misc SAML tests refactors #2727

Merged
merged 5 commits into from
Feb 14, 2024
Merged

Conversation

peterhaochen47
Copy link
Member

@peterhaochen47 peterhaochen47 commented Feb 13, 2024

These are intended to help with the replacement of the SAML extension library.

@cf-gitbot
Copy link

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/187042229

The labels on this github issue will be updated when the story is started.

- this test's prior name ("testSingleLogoutWithLogoutRedirect")
is misleading because it implies similar setup with another test
("testSingleLogout"), where SAML single logout is performed.
But in reality, this test is more similar to another test
("testSingleLogoutWithNoLogoutUrlOnIDP") where SAML single logout
is NOT performed. Hence, renaming the test to reflect that.
- no need to initialize a saml server as we
are using a deployed saml server for this test
- tests are all passing without these setup
- remove unused imports
@peterhaochen47 peterhaochen47 force-pushed the pr/develop/misc-saml-tests-refactors branch from 43f29c2 to 3e89cd0 Compare February 13, 2024 23:46
swalchemist and others added 3 commits February 13, 2024 16:30
* Two tests were not compiling because of one assertion in each, verifying that the SAML filter was not called.
* The structure of the current production code makes the condition the assertions were looking for unlikely to fail.
* We considered adding @ignore on the tests and fixing the assertions later, but decided to keep the tests running and just remove this small bit of test coverage.
* Bruce previously expressed the opinion that we should remove the assertions.

Co-authored-by: Peter Chen <peter-h.chen@broadcom.com>
* The tests pass without the call to samlTestUtils.initializeSimple().
* None of the tests seems to have anything to do with SAML.
@peterhaochen47 peterhaochen47 marked this pull request as ready for review February 14, 2024 00:48
@swalchemist swalchemist requested a review from a team February 14, 2024 00:52
@peterhaochen47 peterhaochen47 merged commit 8cdb674 into develop Feb 14, 2024
20 checks passed
@peterhaochen47 peterhaochen47 deleted the pr/develop/misc-saml-tests-refactors branch February 14, 2024 00:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants