Skip to content

GROOVY-12267: Remove the dead declared-method check from JMX operatio… - #2804

Merged
paulk-asert merged 1 commit into
apache:masterfrom
paulk-asert:groovy12267
Aug 18, 2026
Merged

GROOVY-12267: Remove the dead declared-method check from JMX operatio…#2804
paulk-asert merged 1 commit into
apache:masterfrom
paulk-asert:groovy12267

Conversation

@paulk-asert

Copy link
Copy Markdown
Contributor

…n export

buildOperationMapFrom guarded operation export with

(declaredMethods.contains(name) && !OPS_EXCEPTION_LIST.contains(name))
|| (!OPS_EXCEPTION_LIST.contains(name))

which reduces to its right operand, so the declaredMethods lookup above it was computed and never used and the comment claiming it avoided methods from parents described something the code did not do.

Drop the dead operand and the unused lookup, and state the actual contract in the javadoc: the default export spans the whole inheritance chain, and a caller wanting a narrower set names the operations explicitly in a descriptor.

Behaviour is unchanged. Making the declared-method check live instead was considered and rejected: JmxBuilder's own embedded-descriptor fixtures declare their operations on a base class, and 'operations: ""' routes to this method and has to mean all of them, so restricting to declared methods breaks four existing tests and gives '' a meaning it should not have.

Adds two tests pinning behaviour that was previously untested: inherited operations are exported, and getMetaClass/setMetaClass are not, the latter because they resolve to the metaClass MetaProperty and are dropped by the getter/setter filter.

…n export

buildOperationMapFrom guarded operation export with

  (declaredMethods.contains(name) && !OPS_EXCEPTION_LIST.contains(name))
      || (!OPS_EXCEPTION_LIST.contains(name))

which reduces to its right operand, so the declaredMethods lookup above it
was computed and never used and the comment claiming it avoided methods
from parents described something the code did not do.

Drop the dead operand and the unused lookup, and state the actual contract
in the javadoc: the default export spans the whole inheritance chain, and a
caller wanting a narrower set names the operations explicitly in a
descriptor.

Behaviour is unchanged. Making the declared-method check live instead was
considered and rejected: JmxBuilder's own embedded-descriptor fixtures
declare their operations on a base class, and 'operations: "*"' routes to
this method and has to mean all of them, so restricting to declared methods
breaks four existing tests and gives '*' a meaning it should not have.

Adds two tests pinning behaviour that was previously untested: inherited
operations are exported, and getMetaClass/setMetaClass are not, the latter
because they resolve to the metaClass MetaProperty and are dropped by the
getter/setter filter.
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.1574%. Comparing base (5f56279) to head (6b6b1d0).

Additional details and impacted files

Impacted file tree graph

@@                Coverage Diff                 @@
##               master      #2804        +/-   ##
==================================================
+ Coverage     70.1516%   70.1574%   +0.0058%     
- Complexity      35828      35829         +1     
==================================================
  Files            1562       1562                
  Lines          132523     132522         -1     
  Branches        24379      24379                
==================================================
+ Hits            92967      92974         +7     
+ Misses          31140      31136         -4     
+ Partials         8416       8412         -4     
Files with missing lines Coverage Δ
...groovy/groovy/jmx/builder/JmxMetaMapBuilder.groovy 77.6978% <100.0000%> (+0.2785%) ⬆️

... and 5 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

testlens-app Bot commented Aug 18, 2026

Copy link
Copy Markdown

✅ All tests passed ✅

🏷️ Commit: 6b6b1d0
▶️ Tests: 110479 executed
⚪️ Checks: 31/31 completed


Learn more about TestLens at testlens.app.

@paulk-asert
paulk-asert merged commit e3f320e into apache:master Aug 18, 2026
32 checks passed
@paulk-asert
paulk-asert deleted the groovy12267 branch August 18, 2026 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants