(grails-core) branch test/artefact-name-precomputation updated: Address jdaugherty review feedback on artefact naming characterization spec

[bo...@apache.org]

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 <wb...@mac.com>
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 <no...@anthropic.com>
---
 ...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 {}