Copilot commented on code in PR #16640: URL: https://github.com/apache/grails-core/pull/16640#discussion_r4264430641
########## grails-gsp/grails-layout/src/test/groovy/org/apache/grails/web/layout/LayoutDecoratorSpec.groovy: ########## @@ -0,0 +1,121 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.grails.web.layout + +import com.opensymphony.module.sitemesh.RequestConstants +import com.opensymphony.sitemesh.webapp.SiteMeshWebAppContext +import org.grails.web.util.GrailsApplicationAttributes +import org.grails.web.util.WebUtils +import org.springframework.mock.web.MockHttpServletRequest +import org.springframework.mock.web.MockHttpServletResponse +import org.springframework.web.servlet.View +import spock.lang.Specification + +class LayoutDecoratorSpec extends Specification { + + void 'no decorator writes the original page unchanged'() { + given: + def request = new MockHttpServletRequest() + def response = new MockHttpServletResponse() + def context = new SiteMeshWebAppContext(request, response, request.servletContext) + def html = '<html><head><title>Original</title></head><body>Original body</body></html>' + def content = new GrailsHTMLPageParser().parseContent(html.toCharArray()) + def decorator = new GrailsNoDecorator() + + when: + decorator.render(content, context) + + then: + response.contentAsString == html + decorator.getName() == null + decorator.getPage() == null + decorator.getURIPath() == null + decorator.getRole() == null + decorator.getInitParameter('missing') == null + decorator.getInitParameterNames() == null + } + + void 'view decorator makes page and model available during rendering and removes capture attributes afterwards'() { + given: + def request = new MockHttpServletRequest() + def response = new MockHttpServletResponse() + def content = new GrailsHTMLPageParser().parseContent('<html><body>Body</body></html>'.toCharArray()) + def model = [greeting: 'Hello'] + def view = Mock(View) + def decorator = new SpringMVCViewDecorator('main', view) + + 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') + } Review Comment: Using raw `assert` inside a mock interaction closure can produce less-informative Spock failures and makes it harder to see which condition failed. Prefer capturing the arguments in variables (or using Spock conditions outside the interaction) and asserting them in the `then:` block for clearer reporting. ########## grails-gsp/grails-layout/src/test/groovy/org/apache/grails/web/layout/RenderGrailsLayoutTagLibSpec.groovy: ########## @@ -0,0 +1,92 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.grails.web.layout + +import com.opensymphony.module.sitemesh.RequestConstants +import grails.testing.web.taglib.TagLibUnitTest +import org.grails.buffer.FastStringWriter +import org.grails.plugins.web.taglib.RenderGrailsLayoutTagLib +import spock.lang.Specification + +class RenderGrailsLayoutTagLibSpec extends Specification implements TagLibUnitTest<RenderGrailsLayoutTagLib> { + + GSPGrailsLayoutPage page + + void setup() { + page = new GSPGrailsLayoutPage() + request.setAttribute(RequestConstants.PAGE, page) + } + + void 'pageProperty renders values default values and entire attributes'() { + given: + page.addProperty('body.class', 'content') + + expect: + applyTemplate('<g:pageProperty name="body.class"/>') == 'content' + applyTemplate('<g:pageProperty name="body.class" writeEntireProperty="true"/>') == ' class="content"' + applyTemplate('<g:pageProperty name="missing" default="fallback"/>') == 'fallback' + applyTemplate('<g:pageProperty name="missing"/>') == '' + } + + void 'captured component buffers can be rendered and tested as page properties'() { + given: + def writer = new FastStringWriter() + writer.write('Sidebar') + page.setContentBuffer('sidebar', writer.getBuffer()) + + expect: + applyTemplate('<g:pageProperty name="page.sidebar"/>') == 'Sidebar' + applyTemplate('<g:ifPageProperty name="page.sidebar">Present</g:ifPageProperty>') == 'Present' + } + + void 'ifPageProperty requires existing properties and matching values'() { + given: + page.addProperty('page.theme', 'dark') + page.addProperty('page.mode', 'wide') + + expect: + applyTemplate('<g:ifPageProperty name="page.theme" equals="dark">Yes</g:ifPageProperty>') == 'Yes' + 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>') == '' Review Comment: This assertion intentionally expects no output, but the tag body contains the literal 'No', which can be confusing when scanning the spec. Consider changing the body text to something neutral (e.g., 'SHOULD_NOT_RENDER') or adding a short comment so the intent is unambiguous. ########## grails-gsp/grails-layout/src/test/groovy/org/apache/grails/web/layout/RenderGrailsLayoutTagLibSpec.groovy: ########## @@ -0,0 +1,92 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.grails.web.layout + +import com.opensymphony.module.sitemesh.RequestConstants +import grails.testing.web.taglib.TagLibUnitTest +import org.grails.buffer.FastStringWriter +import org.grails.plugins.web.taglib.RenderGrailsLayoutTagLib +import spock.lang.Specification + +class RenderGrailsLayoutTagLibSpec extends Specification implements TagLibUnitTest<RenderGrailsLayoutTagLib> { + + GSPGrailsLayoutPage page + + void setup() { + page = new GSPGrailsLayoutPage() + request.setAttribute(RequestConstants.PAGE, page) + } + + void 'pageProperty renders values default values and entire attributes'() { Review Comment: Test name reads awkwardly due to duplicated 'values'. Consider renaming for clarity (e.g., include commas and/or remove the repeated word) so failures are easier to interpret in reports. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
