Copilot commented on code in PR #16642: URL: https://github.com/apache/grails-core/pull/16642#discussion_r4264432045
########## grails-gradle/model/src/test/groovy/org/grails/io/support/MainClassFinderSpec.groovy: ########## @@ -0,0 +1,198 @@ +/* + * 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.io.support + +import org.objectweb.asm.ClassWriter +import org.objectweb.asm.Opcodes + +import spock.lang.Specification +import spock.lang.TempDir + +class MainClassFinderSpec extends Specification { + + private static final int PUBLIC_STATIC = Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC + private static final int PRIVATE_STATIC = Opcodes.ACC_PRIVATE | Opcodes.ACC_STATIC + + @TempDir + File tempDir + + private final Set<String> cacheKeys = new HashSet<>() + + void cleanup() { + cacheKeys.each { MainClassFinder.mainClasses.remove(it) } Review Comment: These tests couple to `MainClassFinder`'s internal cache structure (`mainClasses`) and to the precise cache key format (tracked manually via `cacheKeys`). This is brittle: if `MainClassFinder` changes how it keys entries (e.g., absolute vs canonical paths, URI normalization), the suite can start leaking state across tests or failing unexpectedly. Prefer a dedicated test-only cache reset entrypoint (e.g., a package-private `clearCache()` in `MainClassFinder`), or clear the cache in a single operation (rather than removing guessed keys) so isolation doesn’t depend on key formatting. ########## grails-gradle/model/src/test/groovy/org/grails/io/support/UrlResourceSpec.groovy: ########## @@ -0,0 +1,133 @@ +/* + * 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.io.support + +import java.net.MalformedURLException + +import spock.lang.Specification +import spock.lang.TempDir + +class UrlResourceSpec extends Specification { + + @TempDir + File tempDir + + void 'a file resource constructed from a #kind exposes its content and metadata'() { + given: + File file = new File(tempDir, 'message.txt') + file.text = 'URL content' + URI uri = file.toURI() + def resource = kind == 'URI' ? new UrlResource(uri) : + kind == 'URL' ? new UrlResource(uri.toURL()) : new UrlResource(uri.toString()) + + expect: + resource.exists() + resource.isReadable() + resource.inputStream.withCloseable { it.getText('UTF-8') } == 'URL content' + resource.URL == uri.toURL() + resource.URI == uri + resource.file == file + resource.filename == 'message.txt' + resource.contentLength() == file.length() + resource.lastModified() == file.lastModified() + resource.description == "URL [${uri.toURL()}]" + resource.toString() == resource.description + + where: + kind << ['URL', 'URI', 'String'] + } + + void 'relative URLs resolve siblings even when the relative path starts with a slash'() { + given: + File original = new File(tempDir, 'original.txt') + File sibling = new File(tempDir, 'sibling.txt') + sibling.text = 'sibling content' + def resource = new UrlResource(original.toURI()) + + when: + def relative = resource.createRelative(path) + + then: + relative.URL == sibling.toURI().toURL() + relative.inputStream.withCloseable { it.getText('UTF-8') } == 'sibling content' + + where: + path << ['sibling.txt', '/sibling.txt', 'nested/../sibling.txt'] + } + + void 'missing file URLs report absence and fail stream access'() { + given: + def resource = new UrlResource(new File(tempDir, 'missing.txt').toURI()) + + expect: + !resource.exists() + !resource.isReadable() + + when: + resource.getInputStream() + + then: + thrown(FileNotFoundException) + } + + void 'directories are not readable file resources'() { + expect: + new UrlResource(tempDir.toURI()).exists() + !new UrlResource(tempDir.toURI()).isReadable() Review Comment: The test constructs `new UrlResource(tempDir.toURI())` twice in the same assertion block. Consider creating a single `resource` variable and asserting on it, which makes failures easier to diagnose and avoids repeating construction logic. ########## grails-gradle/model/src/test/groovy/org/grails/io/support/GrailsResourceUtilsSpec.groovy: ########## @@ -0,0 +1,265 @@ +/* + * 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.io.support + +import groovy.util.ConfigObject + +import spock.lang.Specification +import spock.lang.TempDir + +class GrailsResourceUtilsSpec extends Specification { + + @TempDir + File tempDir + + void 'source helpers classify #path'() { + expect: + GrailsResourceUtils.getClassName(path) == className + GrailsResourceUtils.isProjectSource(path) == projectSource + GrailsResourceUtils.isGrailsPath(path) == grailsPath + GrailsResourceUtils.isGrailsPath(path) == grailsPath Review Comment: The `isGrailsPath(path)` assertion is duplicated, which reduces signal in this parameterized test and can hide an intended additional assertion. Remove the duplicate line, or replace it with the missing assertion if another behavior was meant to be validated. -- 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]
