jdaugherty commented on code in PR #15478: URL: https://github.com/apache/grails-core/pull/15478#discussion_r2890679408
########## grails-testing-support-httpclient/build.gradle: ########## @@ -0,0 +1,57 @@ +/* + * 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 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. + */ + +plugins { + id 'groovy' + id 'java-library' + id 'org.apache.grails.buildsrc.properties' + id 'org.apache.grails.buildsrc.compile' + id 'org.apache.grails.buildsrc.publish' + id 'org.apache.grails.buildsrc.sbom' + id 'org.apache.grails.gradle.grails-code-style' +} + +version = projectVersion +group = 'org.apache.grails' + +ext { + pomTitle = 'Grails Http Client Testing Support' + pomDescription = Review Comment: I can't make the comment at the folder level, but should we add a README.md with a basic description of this project as well? ########## .github/workflows/groovy-joint-workflow.yml: ########## @@ -159,6 +159,7 @@ jobs: run: > ./gradlew build -x groovydoc + -PgebAtCheckWaiting Review Comment: Well that explains why this has been flakey! ########## grails-testing-support-httpclient/src/main/resources/META-INF/groovy/org.codehaus.groovy.runtime.ExtensionModule: ########## @@ -0,0 +1,3 @@ +moduleName=grails-testing-support-httpclient +moduleVersion=1.0 +extensionClasses=org.apache.grails.testing.httpclient.HttpResponseExtensions Review Comment: Does this mean these extensions would be injected on the code being tested too? Wouldn't that potentially cause a different behavior in test vs prod/dev? Would it be better to use the `@Delegate` pattern and wrap the http response returned by this library so there's no side effect behavior? ########## grails-testing-support-httpclient/src/main/groovy/org/apache/grails/testing/httpclient/HttpClientSupport.groovy: ########## @@ -0,0 +1,418 @@ +/* + * 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.testing.httpclient + +import java.net.http.HttpClient +import java.net.http.HttpRequest +import java.net.http.HttpResponse +import java.time.Duration + +import groovy.json.JsonOutput +import groovy.transform.CompileStatic +import groovy.xml.MarkupBuilder + +import org.springframework.beans.factory.annotation.Value + +/** + * Test support trait that provides a lazily-created JDK {@link HttpClient} for HTTP-based tests. + * <p> + * The client is stored as a static singleton so tests can reuse pooled connections efficiently. + * <p> + * Typical usage in a Grails Spock integration specification (where {@code local.server.port} is injected): + * <pre> + * import grails.testing.mixin.integration.Integration + * import spock.lang.Specification + * + * @Integration + * class MySpec extends Specification implements HttpClientSupport { + * + * void 'health endpoint responds OK'() { + * when: + * def response = http('/health') + * + * then: + * response.expectStatus(200) + * } + * } + * </pre> + * <p> + * Implementing tests must provide {@code local.server.port} (in Grails typically via {@code @Integration}), + * or otherwise override {@code getHttpClientRootUri()}. + */ +@CompileStatic +trait HttpClientSupport { + + @Value('${local.server.port}') + int localServerPort = -1 + + /** + * Shared singleton client reused across tests. + */ + private static volatile HttpClient sharedClient + + /** + * @return the shared singleton client instance, creating it on first access. + */ + HttpClient getHttpClient() { + def client = sharedClient Review Comment: * I'm assuming the HttpClient is thread safe? * I was expecting 1 client per spec class, but this appears to share it across all tests * Why not make use of the Spock infrastructure to create these in a spock extension? ########## grails-doc/src/en/guide/introduction/whatsNew.adoc: ########## @@ -94,3 +94,14 @@ The `g:form` tag now automatically provides csrf protection when Spring Security A Grails banner was introduced in Grails 7 that is displayed on application startup. From Grails 7.1 onwards, this banner now shows versions of foundational dependencies and can be xref:conf.adoc#customizing-the-banner[customized]. + +==== Grails HTTP Client Testing Support (Grails 7.1+) Review Comment: Oof ... I forgot there was a whatsNew doc, and have been adding this stuff to the upgrade guide instead. Good catch. ########## gradle/publish-root-config.gradle: ########## @@ -79,6 +79,7 @@ def publishedProjects = [ 'grails-testing-support-dbcleanup-core', 'grails-testing-support-dbcleanup-h2', 'grails-testing-support-dbcleanup-postgresql', + 'grails-testing-support-httpclient', Review Comment: Trying to think longer term here; I'd like to add the ersatz server as a default configuration as well. This way you can test calling your code & you can test your code calling a mock endpoint. From a naming perspective, should we call this `grails-testing-support-http-client` and then later we'd call the other `grails-testing-support-http-server`? The reason I ask is I can envision eventually having a `grails-testing-support-http-core` that's share between them. -- 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]
