Skip to content

Characterize plugin registry load order and lookup contract - #16004

Merged
jamesfredley merged 2 commits into
8.0.xfrom
test/plugin-registry-contract
Jul 21, 2026
Merged

Characterize plugin registry load order and lookup contract#16004
jamesfredley merged 2 commits into
8.0.xfrom
test/plugin-registry-contract

Conversation

@jamesfredley

Copy link
Copy Markdown
Contributor

Summary

  • add a test-only characterization spec that pins the public plugin-registry contract a future load/registry optimization must preserve
  • asserts the two DISTINCT public observables separately: the actual load sequence consumed by loadPlugins() / getPluginsInLoadOrder() and the getAllPlugins() topological order
  • covers repeated-build determinism, duplicate-registration dedup, and name/class-name/version lookup identity (each resolves the same non-null tail instance)

Scope note

Test-only; no production changes. Exercises only public GrailsPluginManager APIs and deliberately does not duplicate the existing GrailsPluginSorterSpec sorter coverage - it characterizes end-to-end manager registry output instead.

Verification

  • :grails-core:test --tests DefaultGrailsPluginManagerRegistryContractSpec
  • :grails-core:test
  • git diff --check

This is an AI-generated starting point pinning the plugin-registry/load-order contract.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new characterization-style Spock spec to pin down the publicly observable GrailsPluginManager registry behavior around plugin load order vs topological order, including determinism across repeated builds, deduplication, and consistent lookup identity across name/class-name/version APIs. This helps protect expected behavior ahead of future plugin discovery/registry optimizations.

Changes:

  • Introduces DefaultGrailsPluginManagerRegistryContractSpec to assert deterministic plugin discovery load order and deterministic topological registry order for the same dependency graph.
  • Adds coverage for duplicate-registration deduplication and consistent lookup resolution (getGrailsPlugin, getGrailsPluginForClassName, versioned lookup) returning the same instance.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +81 to +83
def tailPlugins = manager.getAllPlugins().findAll { it.name == 'registryOrderingTail' }
def expectedTailPlugin = tailPlugins.first()
def pluginByName = manager.getGrailsPlugin('registryOrderingTail')
@bito-code-review

Copy link
Copy Markdown

To make the selection safe and avoid NoSuchElementException when the list is empty, you can use .find() or .first() with a default value, or check the size before accessing. Given the context of the test, using find { true } or checking the list size is appropriate to ensure the then: block handles the failure reporting correctly.

Here is the suggested update for the test:

        when:
        def tailPlugins = manager.getAllPlugins().findAll { it.name == 'registryOrderingTail' }
        def expectedTailPlugin = tailPlugins ? tailPlugins.first() : null
        def pluginByName = manager.getGrailsPlugin('registryOrderingTail')

This change ensures that expectedTailPlugin is null if the list is empty, allowing the subsequent assertions in the then: block to fail gracefully with descriptive messages rather than throwing an exception.

grails-core/src/test/groovy/grails/plugins/DefaultGrailsPluginManagerRegistryContractSpec.groovy

when:
        def tailPlugins = manager.getAllPlugins().findAll { it.name == 'registryOrderingTail' }
        def expectedTailPlugin = tailPlugins ? tailPlugins.first() : null
        def pluginByName = manager.getGrailsPlugin('registryOrderingTail')

@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 51.4649%. Comparing base (0c63c26) to head (0b53a85).
⚠️ Report is 128 commits behind head on 8.0.x.

Additional details and impacted files

Impacted file tree graph

@@                Coverage Diff                 @@
##                8.0.x     #16004        +/-   ##
==================================================
+ Coverage     51.1168%   51.4649%   +0.3480%     
- Complexity      17597      17753       +156     
==================================================
  Files            2041       2039         -2     
  Lines           95493      95537        +44     
  Branches        16587      16571        -16     
==================================================
+ Hits            48813      49168       +355     
+ Misses          39394      39065       -329     
- Partials         7286       7304        +18     

see 109 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@testlens-app

This comment has been minimized.

Avoid tailPlugins.first(), which throws on an empty list and can mask
Spock then-block failures. Prefer Groovy find (null when missing) and
count for the dedup assertion, matching nearby plugin-spec style.

Assisted-by: Sisyphus:xai/grok-4.5
@jamesfredley
jamesfredley merged commit c6ce3fe into 8.0.x Jul 21, 2026
54 checks passed
@jamesfredley
jamesfredley deleted the test/plugin-registry-contract branch July 21, 2026 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants