fix: fix flaky scaffolding Geb tests and User.getAuthorities() bug#15480
Open
jamesfredley wants to merge 3 commits into7.0.xfrom
Open
fix: fix flaky scaffolding Geb tests and User.getAuthorities() bug#15480jamesfredley wants to merge 3 commits into7.0.xfrom
jamesfredley wants to merge 3 commits into7.0.xfrom
Conversation
Fix User.getAuthorities() which used roles.split('') splitting each
character into a separate authority instead of splitting by comma.
Changed to roles.split(',') with trim() for correct role parsing.
Move login from Spock setup() into each test's when block to avoid
session loss between the lifecycle boundary and the test method,
which caused flaky failures on Java 25 where the browser would see
'Please sign in' instead of the expected page.
Assisted-by: Claude Code <Claude@Claude.ai>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes an incorrect authority parsing implementation in the scaffolding example User domain and stabilizes two flaky Geb integration specs by performing login inside each test rather than in Spock setup().
Changes:
- Fix
User.getAuthorities()to split role strings on commas (and trim entries). - Replace
setup()-based login with anensureLoggedIn()helper called from each spec’swhen:block. - Apply the same test stabilization pattern to both
UserControllerSpecandUserCommunityControllerSpec.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
grails-test-examples/scaffolding/grails-app/domain/com/example/User.groovy |
Adjusts role parsing logic used to build Spring Security GrantedAuthority instances. |
grails-test-examples/scaffolding/src/integrationTest/groovy/com/example/UserControllerSpec.groovy |
Moves login into the feature method to avoid session loss between Spock lifecycle phases. |
grails-test-examples/scaffolding/src/integrationTest/groovy/com/example/UserCommunityControllerSpec.groovy |
Same login timing change to remove intermittent “Please sign in” failures. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...s-test-examples/scaffolding/src/integrationTest/groovy/com/example/UserControllerSpec.groovy
Outdated
Show resolved
Hide resolved
...amples/scaffolding/src/integrationTest/groovy/com/example/UserCommunityControllerSpec.groovy
Outdated
Show resolved
Hide resolved
grails-test-examples/scaffolding/grails-app/domain/com/example/User.groovy
Outdated
Show resolved
Hide resolved
Rename ensureLoggedIn() to loginAsTestUser() for clarity since the method always performs login rather than checking auth state first. Make getAuthorities() null/blank-safe by returning an empty list when roles is null or empty, and filtering out blank entries from split. Assisted-by: Claude Code <Claude@Claude.ai>
jdaugherty
reviewed
Mar 4, 2026
| Collection<? extends GrantedAuthority> getAuthorities() { | ||
| roles.split('').collect { new SimpleGrantedAuthority(it) } | ||
| if (!roles) { | ||
| return Collections.emptyList() |
Contributor
There was a problem hiding this comment.
Why not just '[]' since this is groovy?
| Collection<? extends GrantedAuthority> getAuthorities() { | ||
| roles.split('').collect { new SimpleGrantedAuthority(it) } | ||
| if (!roles) { | ||
| return Collections.emptyList() |
Contributor
There was a problem hiding this comment.
Why not just '[]' since this is groovy?
|
|
||
| void setup() { | ||
| private void loginAsTestUser() { | ||
| to(LoginPage).login() |
Contributor
There was a problem hiding this comment.
Why would the http session be different between setup and the one test? @matrei any idea?
Contributor
There was a problem hiding this comment.
@jdaugherty No, I would think it should work to login in void setup().
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
User.getAuthorities()which incorrectly usedroles.split('')- this split each character of the role string into a separateGrantedAuthority(e.g., 'R', 'O', 'L', 'E', '_', 'U', 'S', 'E', 'R') instead of splitting by comma delimiter. Changed toroles.split(',')withtrim()for correct role parsing.UserControllerSpecandUserCommunityControllerSpecGeb tests. The tests usedsetup()to perform login, but the HTTP session was lost between the Spock lifecycle boundary (setup()-> test method), causing the browser to see "Please sign in" instead of the expected page. Moved login into each test'swhenblock via anensureLoggedIn()helper to eliminate the session gap.CI Failures
This test has been failing intermittently across multiple CI runs on
7.0.x:UserControllerSpec > User list FAILEDUserControllerSpec > User list FAILEDUserControllerSpec > User list FAILEDUserControllerSpec > User list FAILEDChanges
User.groovyroles.split('')->roles.split(',')withtrim()UserControllerSpec.groovysetup()towhenblockUserCommunityControllerSpec.groovysetup()towhenblockTesting
--rerun-tasks -PgebAtCheckWaiting./gradlew codeStyle)