Copilot commented on code in PR #15937: URL: https://github.com/apache/grails-core/pull/15937#discussion_r3558770599
########## grails-testing-support-latency/src/test/groovy/org/apache/grails/testing/latency/LatencyFilterSpec.groovy: ########## @@ -0,0 +1,102 @@ +/* + * 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.latency + +import java.time.Duration + +import spock.lang.Specification + +import org.springframework.mock.web.MockFilterChain +import org.springframework.mock.web.MockHttpServletRequest +import org.springframework.mock.web.MockHttpServletResponse + +class LatencyFilterSpec extends Specification { + + void 'delays the request by at least the configured minimum before continuing the chain'() { + given: + def filter = new LatencyFilter(new LatencyProperties( + minDelay: Duration.ofMillis(100), + maxDelay: Duration.ofMillis(100) + )) + def chain = new MockFilterChain() + + when: + long start = System.nanoTime() + filter.doFilter(new MockHttpServletRequest('GET', '/report/show'), new MockHttpServletResponse(), chain) + long elapsedMillis = Duration.ofNanos(System.nanoTime() - start).toMillis() + + then: + elapsedMillis >= 100 Review Comment: The test converts elapsed time to milliseconds via `Duration.toMillis()`, which truncates; on a 100ms sleep it can report 99ms and make this assertion flaky. Compare in nanoseconds (or add a small tolerance) to avoid rounding-related failures. ########## grails-test-examples/latency/src/integration-test/groovy/latencyapp/LatencyFunctionalSpec.groovy: ########## @@ -0,0 +1,62 @@ +/* + * 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 latencyapp + +import java.time.Duration + +import spock.lang.Specification + +import grails.testing.mixin.integration.Integration +import org.apache.grails.testing.http.client.HttpClientSupport + +/** + * Confirms the behavior of the {@code grails-testing-support-latency} module against a running + * application. The test environment enables latency for {@code /slow/*} with a minimum delay of + * 1500ms (see {@code application.yml}), so matched requests must take at least that long while + * unmatched requests stay well under it. + */ +@Integration +class LatencyFunctionalSpec extends Specification implements HttpClientSupport { + + private static final long MIN_DELAY_MILLIS = 1500 + + void 'requests matching the latency url patterns are delayed by at least the configured minimum'() { + when: + long start = System.nanoTime() + def response = http('/slow/ping') + long elapsedMillis = Duration.ofNanos(System.nanoTime() - start).toMillis() + + then: + response.assertStatus(200) + response.assertContains('pong') + elapsedMillis >= MIN_DELAY_MILLIS Review Comment: Converting nanos to millis via `Duration.toMillis()` truncates, so elapsed time can under-report by 1ms and make the boundary assertion flaky. Compare in nanoseconds (or add a tolerance) to avoid rounding-related failures across platforms/CI. ########## grails-test-examples/latency/src/integration-test/groovy/latencyapp/LatencyFunctionalSpec.groovy: ########## @@ -0,0 +1,62 @@ +/* + * 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 latencyapp + +import java.time.Duration + +import spock.lang.Specification + +import grails.testing.mixin.integration.Integration +import org.apache.grails.testing.http.client.HttpClientSupport + +/** + * Confirms the behavior of the {@code grails-testing-support-latency} module against a running + * application. The test environment enables latency for {@code /slow/*} with a minimum delay of + * 1500ms (see {@code application.yml}), so matched requests must take at least that long while + * unmatched requests stay well under it. + */ +@Integration +class LatencyFunctionalSpec extends Specification implements HttpClientSupport { + + private static final long MIN_DELAY_MILLIS = 1500 + + void 'requests matching the latency url patterns are delayed by at least the configured minimum'() { + when: + long start = System.nanoTime() + def response = http('/slow/ping') + long elapsedMillis = Duration.ofNanos(System.nanoTime() - start).toMillis() + + then: + response.assertStatus(200) + response.assertContains('pong') + elapsedMillis >= MIN_DELAY_MILLIS + } + + void 'requests outside the latency url patterns are not delayed'() { + when: + long start = System.nanoTime() + def response = http('/fast/ping') + long elapsedMillis = Duration.ofNanos(System.nanoTime() - start).toMillis() + + then: + response.assertStatus(200) + response.assertContains('pong') + elapsedMillis < MIN_DELAY_MILLIS Review Comment: This assertion has the same truncation risk as the delayed-path test: `toMillis()` can under-report elapsed time by 1ms. Switching to nanosecond comparisons removes rounding artifacts. ########## grails-testing-support-latency/src/main/groovy/org/apache/grails/testing/latency/LatencyFilter.groovy: ########## @@ -0,0 +1,93 @@ +/* + * 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.latency + +import groovy.transform.CompileStatic +import groovy.util.logging.Slf4j +import jakarta.servlet.FilterChain +import jakarta.servlet.ServletException +import jakarta.servlet.http.HttpServletRequest +import jakarta.servlet.http.HttpServletResponse + +import org.springframework.web.filter.OncePerRequestFilter + +/** + * Servlet filter that delays each matched request by a random amount of time before + * passing it down the filter chain. + * <p> + * Slowing responses down widens the window in which timing bugs occur, turning + * intermittent functional test flakiness (assertions that race a navigation, missing + * waits, ordering assumptions) into deterministic failures. It is intended purely as a + * testing aid and should never be enabled in production. + * + * @see LatencyAutoConfiguration + * @see LatencyProperties + */ +@Slf4j +@CompileStatic +class LatencyFilter extends OncePerRequestFilter { + + private final long minDelayMillis + private final long maxDelayMillis + private final double probability + private final Random random + + LatencyFilter(LatencyProperties properties) { + minDelayMillis = properties.minDelay.toMillis() + maxDelayMillis = properties.maxDelay.toMillis() + probability = properties.probability + if (minDelayMillis < 0) { + throw new IllegalArgumentException("grails.testing.latency.min-delay must not be negative, was ${properties.minDelay}") + } + if (maxDelayMillis < minDelayMillis) { + throw new IllegalArgumentException("grails.testing.latency.max-delay (${properties.maxDelay}) " + + "must not be less than min-delay (${properties.minDelay})") + } + if (probability < 0.0d || probability > 1.0d) { + throw new IllegalArgumentException("grails.testing.latency.probability must be between 0.0 and 1.0, was $probability") + } Review Comment: `probability` validation doesn’t reject NaN/Infinity (both bypass the current `< 0` / `> 1` checks). Treat non-finite values as invalid so misconfiguration fails fast as intended. -- 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]
