Fix under-determined test: testWithParam

[KiruthikaJanakiraman]

Checklist

• [x] the individual contributor license agreement is signed
• [x] commit message follows commit guidelines

Description of change

This PR aims to fix an under-determined test: org.apereo.portal.url.CasLoginRefUrlEncoderTest.testWithParam

I found and confirmed the failure of the test using an open-source research tool NonDex, which shuffles implementations of nondeterminism operations.

Problem The test case org.apereo.portal.url.CasLoginRefUrlEncoderTest.testWithParam fails due to the below assertion:

https://github.com/uPortal-Project/uPortal/blob/23e387d65fd4f740533b1d27d840632724582f43/uPortal-security/uPortal-security-mvc/src/test/java/org/apereo/portal/url/CasLoginRefUrlEncoderTest.java#L66-L68

This is because, findMatchingServerName returns the first element from allServerNames collection.

https://github.com/uPortal-Project/uPortal/blob/5d1d7436a24c3b435e26798f9346ec8c7c539c3a/uPortal-utils/uPortal-utils-url/src/main/java/org/apereo/portal/url/UrlMultiServerNameCustomizer.java#L63

Since allServerNames is a HashSet, the order of the items "myhost:8080", "theirhost:8443" in the HashSet is not preserved. Hence the server name returned by findMatchingServerName may vary which may cause the test testWithParam to fail in a hypothetical future.

Solution This PR fixes the test by using a LinkedHashSet to store allServerNames since LinkedHashSet preserves the order of the elements.

Steps to reproduce The following command can be used to reproduce the assertion errors and verify the fix.

Integrate NonDex:

Add the following snippet to the top of the build.gradle in $PROJ_DIR:

plugins {
    id 'edu.illinois.nondex' version '2.1.1-1'
}

Append to build.gradle in $PROJ_DIR:

apply plugin: 'edu.illinois.nondex'

Execute Test with Gradle:

./gradlew --info uPortal-security:test --tests org.apereo.portal.url.CasLoginRefUrlEncoderTest.testWithParam

Run NonDex:

./gradlew --info uPortal-security:nondexTest --tests=org.apereo.portal.url.CasLoginRefUrlEncoderTest.testWithParam --nondexRuns=50 -x autoLintGradle

Test Environment:

Java version "1.8.0_381"
macOS Venture Version 13.4.1 (22F82)

Please let me know if you have any concerns or questions.

[ChristianMurphy]

Hey @KiruthikaJanakiraman 👋

This is because, findMatchingServerName returns the first element from allServerNames collection.

I'm not sure I follow, findMatchingServerName returns the first match not the first element. Both in the test and in most real world scenarios there will be no overlapping servernames so only one match will be returned.

[KiruthikaJanakiraman]

Hi,

If I'm not wrong, the case where comparisonHost is empty, the function returns the first element of allServerNames: https://github.com/uPortal-Project/uPortal/blob/5d1d7436a24c3b435e26798f9346ec8c7c539c3a/uPortal-utils/uPortal-utils-url/src/main/java/org/apereo/portal/url/UrlMultiServerNameCustomizer.java#L63

[ChristianMurphy]

@jgribonvald do you have insights on the intent of this method?