Copilot commented on code in PR #16638: URL: https://github.com/apache/grails-core/pull/16638#discussion_r4264430457
########## grails-core/src/test/groovy/org/grails/config/CompositeConfigSpec.groovy: ########## @@ -0,0 +1,184 @@ +/* + * 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.grails.config + +import org.grails.core.exceptions.GrailsConfigurationException +import spock.lang.Specification + +class CompositeConfigSpec extends Specification { + + void 'first config takes precedence and last config supplies missing values'() { + given: + def config = new CompositeConfig() + config.addLast(new PrefixedConfig('scope', new PropertySourcesConfig([scope: [shared: 'middle', fallback: 42]]))) + config.addLast(new PrefixedConfig('scope', new PropertySourcesConfig([scope: [shared: 'last', lastOnly: 'last']]))) + config.addFirst(new PrefixedConfig('scope', new PropertySourcesConfig([scope: [shared: 'first', firstOnly: 'first']]))) + + expect: + config.getAt('shared') == 'first' + config.get('shared') == 'first' + config.getProperty('shared') == 'first' + config.getProperty('fallback', Integer) == 42 + config.getAt('lastOnly') == 'last' + config.getAt('firstOnly') == 'first' + config.getAt('missing') == null + config.get('missing') == null + config.getProperty('missing', Integer) == null + config.getProperty('missing', Integer, 7) == 7 + config.getProperty('missing', 'default') == 'default' + } + + void 'navigate searches nested paths across configs'() { + given: + def config = new CompositeConfig() + config.addFirst(new PropertySourcesConfig([service: [host: 'first']])) + config.addLast(new PropertySourcesConfig([service: [host: 'last', port: 8080]])) + + expect: + config.navigate('service', 'host') == 'first' + config.navigate('service', 'port') == 8080 + config.navigate('service', 'missing') == null + } + + void 'flatten and properties combine configs in insertion order'() { + given: + def config = new CompositeConfig() + config.addFirst(new PropertySourcesConfig([shared: 'first', firstOnly: 'one'])) + config.addLast(new PropertySourcesConfig([shared: 'last', lastOnly: 'two'])) + + expect: + config.flatten() == [shared: 'last', firstOnly: 'one', lastOnly: 'two'] + config.toProperties() == [shared: 'last', firstOnly: 'one', lastOnly: 'two'] as Properties + config.getProperty('shared') == 'first' + } + + void 'map views include every config and deduplicate keys'() { + given: + def first = new PropertySourcesConfig([shared: 'first', firstOnly: 'one']) + def last = new PropertySourcesConfig([shared: 'last', lastOnly: 'two']) + def config = new CompositeConfig() + config.addFirst(first) + config.addLast(last) + + expect: + config.size() == first.size() + last.size() + !config.isEmpty() + config.keySet() == ['shared', 'firstOnly', 'lastOnly'] as Set + config.containsKey('firstOnly') + config.containsKey('lastOnly') + !config.containsKey('missing') + config.containsProperty('shared') + !config.containsProperty('missing') + config.containsValue('first') + config.containsValue('last') + !config.containsValue('missing') + config.values().toSet() == ['first', 'last', 'one', 'two'] as Set + config.entrySet() == (first.entrySet() + last.entrySet()) as Set + config.iterator().toList().toSet() == config.entrySet() Review Comment: These assertions are internally inconsistent with `Map` semantics and with the earlier 'deduplicate keys' intent. `keySet()` asserts 3 unique keys, but `size()` asserts the sum of delegate sizes (which double-counts `shared`), and `entrySet()` compares against a set containing two different entries for the same key (`shared`)—which a `Map.entrySet()` should not contain. Consider asserting `size() == config.keySet().size()` and verifying `entrySet()`/`iterator()` against a de-duplicated view (e.g., derived from `flatten()` or by mapping `keySet()` to resolved `getAt(key)` values). ########## grails-core/src/test/groovy/grails/util/GrailsClassUtilsSpec.groovy: ########## @@ -0,0 +1,196 @@ +/* + * 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 grails.util + +import java.io.ByteArrayInputStream +import java.io.Closeable + +import grails.artefact.Enhanced +import groovy.util.ConfigObject +import spock.lang.Specification + +class GrailsClassUtilsSpec extends Specification { + + void 'interface discovery includes superclass and superinterface declarations'() { + given: + def instance = new ByteArrayInputStream(new byte[0]) + def expected = [Closeable, AutoCloseable] as Set + + expect: + (GrailsClassUtils.getAllInterfaces(instance) as Set) == expected + GrailsClassUtils.getAllInterfacesAsSet(instance) == expected + (GrailsClassUtils.getAllInterfacesForClass(ByteArrayInputStream) as Set) == expected + (GrailsClassUtils.getAllInterfacesForClass(ByteArrayInputStream, getClass().classLoader) as Set) == expected + GrailsClassUtils.getAllInterfacesForClassAsSet(ByteArrayInputStream) == expected + GrailsClassUtils.getAllInterfacesForClassAsSet(ByteArrayInputStream, getClass().classLoader) == expected + GrailsClassUtils.getAllInterfacesForClass(Object).length == 0 + } + + void 'interface discovery rejects null arguments'() { + when: + operation() + + then: + thrown(IllegalArgumentException) + + where: + operation << [ + { GrailsClassUtils.getAllInterfaces(null) }, + { GrailsClassUtils.getAllInterfacesAsSet(null) }, + { GrailsClassUtils.getAllInterfacesForClassAsSet(null) } + ] + } + + void 'visibility checks accept visible classes and reject unavailable classes'() { + given: + def bootstrapOnly = new ClassLoader(null) { } + + expect: + GrailsClassUtils.isVisible(TestBean, null) + GrailsClassUtils.isVisible(TestBean, getClass().classLoader) + GrailsClassUtils.isVisible(String, bootstrapOnly) + !GrailsClassUtils.isVisible(TestBean, bootstrapOnly) + } + + void 'property descriptors expose property types and exact type matching'() { + expect: + GrailsClassUtils.getPropertyType(TestBean, 'name') == String + GrailsClassUtils.getPropertyType(TestBean, 'missing') == null + GrailsClassUtils.getPropertyType(null, 'name') == null + GrailsClassUtils.getPropertyType(TestBean, ' ') == null + GrailsClassUtils.isPropertyOfType(TestBean, 'name', String) + !GrailsClassUtils.isPropertyOfType(TestBean, 'name', CharSequence) + !GrailsClassUtils.isPropertyOfType(TestBean, 'missing', String) + GrailsClassUtils.getProperty(TestBean, 'name').propertyType == String + GrailsClassUtils.getProperty(TestBean, 'name', String).name == 'name' + GrailsClassUtils.getProperty(TestBean, 'name', Integer) == null + GrailsClassUtils.getProperty(TestBean, 'missing') == null + GrailsClassUtils.getProperty(null, 'name') == null + GrailsClassUtils.getProperty(TestBean, null) == null + GrailsClassUtils.getProperty(TestBean, 'name', null) == null + } + + void 'property type selection distinguishes assignable properties from accepting properties'() { + expect: + GrailsClassUtils.getPropertiesOfType(TestBean, String)*.name.toSet() == ['name', 'text'] as Set + GrailsClassUtils.getPropertiesAssignableToType(TestBean, String)*.name == ['name'] + GrailsClassUtils.getPropertiesAssignableToType(TestBean, CharSequence)*.name.toSet() == ['name', 'text'] as Set + GrailsClassUtils.getPropertiesOfType(null, String).length == 0 + GrailsClassUtils.getPropertiesOfType(TestBean, null).length == 0 + GrailsClassUtils.getPropertiesAssignableToType(null, String).length == 0 + GrailsClassUtils.getPropertiesAssignableToType(TestBean, null).length == 0 + } + + void 'new instance property lookup handles valid invalid and uninstantiable classes'() { Review Comment: Fix grammar in the test name: 'valid invalid' should be 'valid, invalid' (or 'valid/invalid') to read unambiguously. ########## grails-core/src/test/groovy/org/grails/core/util/StopWatchSpec.groovy: ########## @@ -0,0 +1,150 @@ +/* + * 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.grails.core.util + +import spock.lang.Specification + +class StopWatchSpec extends Specification { + + void 'new watches have no tasks or elapsed time'() { + given: + def watch = new StopWatch() + + expect: + !watch.isRunning() + watch.getTaskCount() == 0 + watch.getTaskInfo().length == 0 + watch.getTotalTimeMillis() == 0 + watch.getTotalTimeSeconds() == 0.0 + watch.shortSummary() == "StopWatch '': running time (millis) = 0" + watch.prettyPrint().contains('Task name') + watch.toString() == watch.shortSummary() + } + + void 'start and stop record an unnamed task and complete ends the watch'() { + given: + def watch = new StopWatch('startup') + + when: + watch.start() + + then: + watch.isRunning() + watch.getTaskCount() == 1 + watch.getLastTaskName() == '' + + when: + watch.stop() + watch.complete() + + then: + !watch.isRunning() + watch.getTaskInfo().length == 1 + watch.getLastTaskTimeMillis() >= 0 + watch.getLastTaskInfo().is(watch.getTaskInfo()[0]) + watch.getTotalTimeMillis() == watch.getLastTaskTimeMillis() + watch.getTotalTimeSeconds() == watch.getTotalTimeMillis() / 1000.0 + watch.getLastTaskInfo().getTimeSeconds() == watch.getLastTaskTimeMillis() / 1000.0 + } + + void 'multiple named tasks contribute to totals and reports'() { + given: + def watch = new StopWatch('startup') + + when: + watch.start('configuration') + watch.stop() + watch.start('beans') + watch.stop() + watch.complete() + + then: + watch.getTaskCount() == 2 + watch.getTaskInfo()*.taskName == ['configuration', 'beans'] + watch.getLastTaskName() == 'beans' + watch.getTotalTimeMillis() == watch.getTaskInfo()*.timeMillis.sum() + watch.getTaskInfo().every { it.timeMillis >= 0 } + watch.shortSummary() == "StopWatch 'startup': running time (millis) = ${watch.getTotalTimeMillis()}" + watch.prettyPrint().startsWith(watch.shortSummary()) + watch.prettyPrint().contains('ms % Task name') + watch.prettyPrint().contains('configuration\n') + watch.prettyPrint().contains('beans\n') Review Comment: These assertions make the test brittle to report formatting changes (exact spacing) and platform line separators (`\\n` vs `\\r\\n`). To reduce flaky failures across environments and future formatting tweaks, prefer checking for stable tokens (e.g., column headers without spacing) and task names without embedding a specific newline, or normalize line endings before asserting. -- 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]
