Repository navigation
Add unit tests for Grails layout capture and response buffering - #16640
Merged
Merged
Conversation
Cover page wrappers, HTML parsing, decorators, and layout tag capture. Assisted-by: sisyphus-grok:grok-4.7
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
3 open findings
assertinside a mock interaction closure can produce less-informative Spock failures… · New
What changed in this PR
Adds Spock specs to improve coverage of the Grails layout module, focusing on response buffering, HTML parsing, decorators, and layout tag capture.
Changes:
- Added unit specs for layout taglibs (
GrailsLayoutTagLib,RenderGrailsLayoutTagLib) covering capture and page property behavior. - Added unit specs for layout rendering/decorators and response wrappers/buffering behavior.
- Added unit specs for the HTML page parser and
GSPGrailsLayoutPagebuffer/reset behavior.
| File | Description |
|---|---|
| grails-gsp/grails-layout/src/test/groovy/org/apache/grails/web/layout/RenderGrailsLayoutTagLibSpec.groovy | New taglib unit tests for page properties and layout tag rendering. |
| grails-gsp/grails-layout/src/test/groovy/org/apache/grails/web/layout/LayoutDecoratorSpec.groovy | New tests for decorator rendering behavior and cleanup semantics. |
| grails-gsp/grails-layout/src/test/groovy/org/apache/grails/web/layout/GrailsViewBufferingResponseSpec.groovy | New tests validating view buffering behavior across writer/stream and error/redirect. |
| grails-gsp/grails-layout/src/test/groovy/org/apache/grails/web/layout/GrailsPageResponseWrapperSpec.groovy | New tests for response wrapper buffering/parsing, pass-through, and dispatch delegation. |
| grails-gsp/grails-layout/src/test/groovy/org/apache/grails/web/layout/GrailsLayoutTagLibSpec.groovy | New tests for capture tags storing head/body/title/meta/content into the layout page. |
| grails-gsp/grails-layout/src/test/groovy/org/apache/grails/web/layout/GrailsHTMLPageParserSpec.groovy | New tests for parsing title/head/body/properties and Content interface behavior. |
| grails-gsp/grails-layout/src/test/groovy/org/apache/grails/web/layout/GSPGrailsLayoutPageSpec.groovy | New tests for buffer semantics, title capture stripping, “used” flags, and reset. |
🧠 Review effort: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+63
to
+72
|
|
||
| when: | ||
| decorator.render(content, model, request, response, request.servletContext) | ||
|
|
||
| then: | ||
| 1 * view.render(model, request, response) >> { Map values, servletRequest, servletResponse -> | ||
| assert servletRequest.getAttribute(RequestConstants.PAGE).body == 'Body' | ||
| assert servletRequest.getAttribute(EmbeddedGrailsLayoutView.GSP_GRAILS_LAYOUT_PAGE) instanceof GSPGrailsLayoutPage | ||
| servletResponse.getWriter().write(values.greeting + ': Body') | ||
| } |
| request.setAttribute(RequestConstants.PAGE, page) | ||
| } | ||
|
|
||
| void 'pageProperty renders values default values and entire attributes'() { |
Comment on lines
+66
to
+68
| applyTemplate('<g:ifPageProperty name="page.theme" equals="light">No</g:ifPageProperty>') == '' | ||
| applyTemplate('<g:ifPageProperty name="missing">No</g:ifPageProperty>') == '' | ||
| applyTemplate('<g:ifPageProperty>No</g:ifPageProperty>') == '' |
Copilot stopped reviewing on behalf of
jamesfredley due to an error
October 14, 2026 00:36
Capture the request attributes seen during layout rendering into variables and assert on them in the then block, so a failure reports which condition broke instead of surfacing as a bare assert inside the mock interaction. Cover the title buffer path of GSPGrailsLayoutPage.writeHead through the wrapTitleTag and captureTitle tags, the preprocessor's XML closing marker on an empty capture tag, wrapTitleTag without a layout page, pageProperty without its required name attribute, and sendError being skipped once the target response is committed. Rename the pageProperty feature and use neutral body text in the negative ifPageProperty cases so the expected empty output reads as intended.
🔎 No tests executed 🔎🏷️ Commit: 9f8c9b8 Learn more about TestLens at testlens.app/docs. |
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
Test plan
./gradlew :grails-layout:testfor the new specs (42 tests, 0 failures)