This is an automated email from the ASF dual-hosted git repository.
borinquenkid pushed a commit to branch test/artefact-name-precomputation
in repository https://gitbox.apache.org/repos/asf/grails-core.git
The following commit(s) were added to
refs/heads/test/artefact-name-precomputation by this push:
new bfe84593dd Address jdaugherty review feedback on artefact naming
characterization spec
bfe84593dd is described below
commit bfe84593dd2ab8a1808372781fdbae39363ccaf0
Author: Walter Duque de Estrada <[email protected]>
AuthorDate: Mon Jul 27 12:33:16 2026 -0500
Address jdaugherty review feedback on artefact naming characterization spec
Copilot's 5 inline comments claiming the acronym naturalName expectations
were wrong (e.g. 'HTMLC ontroller') are false positives - verified by
running the spec: GrailsNameUtils.getNaturalName genuinely produces those
values, and jdaugherty's review already confirmed this. No change needed
there.
jdaugherty's own feedback was substantive and is addressed here:
- Rename ArtefactNamePrecomputationSpec -> ArtefactNamingContractSpec:
"precomputed" was aspirational, since nothing in the spec exercises
actual precomputation, only naming stability.
- Add a comment above the acronym-heavy naturalName assertions explaining
the quirky-but-intentional GrailsNameUtils splitting behavior they pin,
so a future reader doesn't "fix" the expectations or the algorithm
without realizing this spec exists to catch exactly that change.
- Add three cases exercising the artefact detection contract
(ArtefactHandler#isArtefactClass), which the original spec bypassed
entirely by constructing GrailsClass wrappers directly: an abstract
controller is rejected (ControllerArtefactHandler's allowAbstract is
false), a suffix-matching concrete controller is accepted, and a
domain-named class with no @Entity/@Artefact annotation is rejected by
DomainClassArtefactHandler - this is the part a naming precomputation
refactor is most likely to disturb, and the prior spec gave it no
coverage at all.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
---
...ec.groovy => ArtefactNamingContractSpec.groovy} | 33 ++++++++++++++++++++--
1 file changed, 30 insertions(+), 3 deletions(-)
diff --git
a/grails-core/src/test/groovy/org/grails/core/ArtefactNamePrecomputationSpec.groovy
b/grails-core/src/test/groovy/org/grails/core/ArtefactNamingContractSpec.groovy
similarity index 83%
rename from
grails-core/src/test/groovy/org/grails/core/ArtefactNamePrecomputationSpec.groovy
rename to
grails-core/src/test/groovy/org/grails/core/ArtefactNamingContractSpec.groovy
index 4c383dea6d..f04c7bae60 100644
---
a/grails-core/src/test/groovy/org/grails/core/ArtefactNamePrecomputationSpec.groovy
+++
b/grails-core/src/test/groovy/org/grails/core/ArtefactNamingContractSpec.groovy
@@ -19,21 +19,31 @@
package org.grails.core
import grails.core.GrailsClass
+import org.grails.core.artefact.ControllerArtefactHandler
+import org.grails.core.artefact.DomainClassArtefactHandler
import spock.lang.Specification
import spock.lang.Unroll
/**
- * Tests for deterministic GrailsClass naming metadata.
+ * Tests for deterministic GrailsClass naming metadata, and for the artefact
detection
+ * contract (ArtefactHandler#isArtefactClass) that a naming precomputation
refactor is
+ * most likely to disturb.
*/
-class ArtefactNamePrecomputationSpec extends Specification {
+class ArtefactNamingContractSpec extends Specification {
private static final String BASE_PACKAGE = 'org.grails.core'
@Unroll
- void "naming contract for #entry.label is precomputed"() {
+ void "#entry.label naming metadata matches the observed contract"() {
given:
GrailsClass grailsClass = grailsClassFor(entry.artifactType,
entry.wrapperClass)
+ // GrailsNameUtils.getNaturalName splits into words at each
lower-to-upper case
+ // transition, but an acronym run (HTML, JSONAPI, URL) absorbs the
first letter
+ // of the following word before that transition triggers - hence
'HTMLC ontroller'
+ // (not 'HTML Controller') and 'JSONAPIS ervice' (not 'JSONAPI
Service') below.
+ // These are pinned intentionally: don't "fix" these expectations, or
the
+ // algorithm, without knowing this spec exists to catch exactly that
change.
expect:
grailsClass.name == entry.name
grailsClass.shortName == entry.shortName
@@ -186,6 +196,21 @@ class ArtefactNamePrecomputationSpec extends Specification
{
]
}
+ void "ControllerArtefactHandler accepts a concrete class whose name ends
with the controller suffix"() {
+ expect:
+ new ControllerArtefactHandler().isArtefactClass(HTMLController)
+ }
+
+ void "ControllerArtefactHandler rejects an abstract class even when the
name matches the controller suffix"() {
+ expect:
+ !new ControllerArtefactHandler().isArtefactClass(AbstractFooController)
+ }
+
+ void "DomainClassArtefactHandler rejects a class named like a domain class
but carrying no domain annotation"() {
+ expect:
+ !new DomainClassArtefactHandler().isArtefactClass(URLDomain)
+ }
+
private GrailsClass grailsClassFor(String artifactType, Class<?>
wrapperClass) {
switch (artifactType) {
case 'controller':
@@ -204,6 +229,8 @@ class ArtefactNamePrecomputationSpec extends Specification {
class HTMLController {}
+abstract class AbstractFooController {}
+
class PayRollController {}
class JSONAPIService {}
