test(config): add reference.conf to bean parity gate#6803
Open
bladehan1 wants to merge 1 commit into
Open
Conversation
Add ConfigParityGateTest as the single build-time gate validating that
reference.conf and its *Config beans stay in lockstep. Five sub-gates
run against every entry in SECTIONS:
1) hoconKeysAreBound — every HOCON key has a matching writable bean
property or is in the per-section HOCON-orphan allowlist with a
rationale comment.
2) beanPropertiesHaveHoconKeys — every writable bean property has a
matching HOCON key or is in the per-section bean-orphan allowlist.
3) defaultValuesMatch — every supported-type bean property has a
default value equal to the reference.conf value; types outside the
dispatcher matrix are a hard failure (no silent escape).
4) allowlistEntriesAreLive — every per-section allowlist entry still
resolves to a live HOCON path or writable bean property; prevents
allowlist rot when a key is renamed or removed.
5) everyReferenceConfTopLevelKeyIsCovered — meta-gate closing the
"new top-level section sneaks in" hole.
ConfigParityCheck holds the shared recursive walkers, type dispatcher,
shape-mismatch guard (a nested *Config property must see a HOCON
OBJECT), and per-section + cross-section accounting. Default-value
mismatch messages stamp both sides with the runtime type so that an
Integer(10) vs Long(10) divergence is no longer visually identical.
Scope: gates validate only the SECTIONS bucket. Top-level keys outside
SECTIONS (crypto, enery, localwitness, net, seed, trx) are counted for
file-coverage alignment but their subtrees are not value-validated —
they hang off MiscConfig manual-read paths or non-bean roots.
Each helper provides a (label, Config, ...) overload that walks a
caller-supplied Config without bumping the production AGGREGATES, so
unit tests of the gate itself stay isolated from the gate's own
coverage totals.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Adds a single build-time parity gate between
reference.confand the*Configbeans bound byConfigBeanFactory. Two new files undercommon/src/test/java/org/tron/core/config/args/:ConfigParityGateTest— JUnit class holding theSectionregistry (path → bean → per-section allowlists) and the@Testmethods that iterate every registered section.ConfigParityCheck— shared helpers: recursive HOCON / bean walkers, scalar type dispatcher, allowlist plumbing, and cross-section aggregate logging.Five sub-gates run for every entry in the registry:
hoconKeysAreBound— every HOCON key under<section>.*has a writable bean property (recursing into nested*Configbeans) or is on the per-section HOCON-orphan allowlist with an inline rationale.beanPropertiesHaveHoconKeys— every writable bean property has a matching HOCON key (recursing the same way) or is on the per-section bean-orphan allowlist.defaultValuesMatch— every supported-type bean property's default value equals thereference.confvalue. Nested*Configbeans are recursed into; types outside the scalar matrix hard-fail with no allowlist escape.allowlistEntriesAreLive— every per-section allowlist entry (hoconOrphans/beanOrphans/divergent) still resolves to a live HOCON path or writable bean property. Catches the rot mode where a key/property is renamed or removed but the grandfathering entry is left behind — without this gate, the rationale comment would silently outlive the underlying anomaly (the long-term failure mode of Cassandra-stylePROPERTIES_TO_IGNOREblacklists).A meta-gate (
everyReferenceConfTopLevelKeyIsCovered) ensures every top-levelreference.confkey is either covered by a registeredSectionor explicitly listed inTOP_LEVEL_NON_BEANwith a rationale (crypto,enery,localwitness,net,seed,trx).Registered sections:
block,committee,event.subscribe,genesis.block,node,node.metrics,rate.limiter,storage,vm.Why are these changes required?
ConfigBeanFactory.create(...)silently drops HOCON keys that have no matching setter, and silently retains bean defaults that diverge fromreference.conf. A unit-test gate is the cheapest place to catch this drift at PR time rather than at process startup (or worse, in production).The gate's class Javadoc also documents two naming conventions required for new keys to bind without a
normalizeNonStandardKeys()hook:allowPbft, notallowPBFT;httpUrl, notHTTPURL. Avoids the "first two characters preserved on decapitalize" trap that producedpBFTExpireNum.isXxxboolean keys — preferenableXxxorxxxSwitch. Theisprefix forces a manual rename hook.The v4.8.2 baseline (acronym casing,
isOpenFullTcpDisconnect,event.subscribe.native/topics) is grandfathered via per-section allowlists with inline rationale; the policy is "do not extend without explicit reason."This PR has been tested by:
./gradlew :common:test --tests "*ConfigParityGateTest*"→ all five sub-gates green (hoconKeysAreBound,beanPropertiesHaveHoconKeys,defaultValuesMatch,allowlistEntriesAreLive,everyReferenceConfTopLevelKeyIsCovered). The[parity-summary]block verifies the two coverage invariants:file-hoconKey = checkSection + cantCheckSectionregistry-beanKey = parity-bean = parity-default:common:test— 108 tests across 12 suites green, including all per-section*ConfigTestbehavioural tests alongside the new parity gate.ConfigParityCheck's four assertion helpers (one fixture.conf+ one synthetic*Config-shaped bean per branch). Sources live outsidesrc/to keep this team CI gate dependency-free; a runner copies them in, executes Gradle, and tears down. Matrix:assertNoHoconOrphans— happy path, top-level orphan, allowlist hit, level-2 orphan via recursion, shape mismatch (child is scalar), recursion-filter rejects non-org.tron.*type.assertNoBeanOrphans— happy path, property without key, level-2 bean orphan via recursion, shape mismatch.assertDefaultValuesMatch— happy path, per-scalar drift (int / long / boolean / double / float / String / List), allowlist hit, nested recursion happy + drift, shape-mismatch guard (recursing into a non-OBJECT HOCON value previously threwConfigException.WrongType; now surfaces a clean field-pointing mismatch), unsupported Java type (Map), write-only property (setter, no getter), null nested field, type-coerce incompatible.assertAllowlistEntriesAreLive— every-entry-resolves, deadhoconOrphan, deadbeanOrphan, divergent dead on hocon side, divergent dead on bean side, nested dotted ok + nested dotted bean-side dead (exercisesbeanPropertyExists' dotted-recursion path).config/checkstyle/checkStyleAll.xmlpasses on both new files, 0 violations.Follow up
*Configbean introduces a Map/enum/Duration/MemorySize field, re-introduce aSection.typeSkipfield and the correspondingassertDefaultValuesMatchparameter following the comment block inSection.// ...comments onreference.confso every leaf key carries a one-line description of intent / units / interaction; (b) add a sub-gate that fails when a registered-section leaf key has no preceding comment. Step (b) is held until step (a) lands — gating an under-commented file would force vacuous one-liners and defeat the point.Extra details
Design tradeoffs — five gate ideas were considered. We deliberately ship only the four shipped here and reject / defer the rest:
Config.get*) and the contract surface is small and explicit.applyXxxConfigassigns every bean field toParametergetXxxhas a production consumerreference.confis commented by logical block, not per-leaf, so a coverage gate over the current file is noise. Plan is to first complete the inline comments (every leaf key gets a one-line description of intent / units / interaction), then land the gate. Tracked under Follow up above.*ConfigTestassertEquals(x, x)boilerplate. Coverage belongs to JaCoCo + review.Scope discipline that fell out of this:
*ConfigTestbehavioural cases (alias fallback, clamp,fromConfigoutputs) plus integration tests.MiscConfig,LocalWitnessConfig) are intentionally out of scope. Their HOCON paths are scattered top-level keys with no 1:1 bean field, so any "parity gate" would mean asserting a hand-written mapping that duplicates the very thing it's supposed to protect, and could not catch the actual failure mode (a new manual-read key being added without sync).TOP_LEVEL_NON_BEANaccounts for them as "explicitly out-of-scope with rationale" instead.Duration,MemorySize, custom classes) are a hard failure today rather than an allowlistable skip. None of the current*Configbeans use such types; the comment inSectiondocuments the path for re-introducing atypeSkipfield if a future bean genuinely cannot be value-compared.Implementation notes:
Sectionrow per (path, bean, hoconOrphans, beanOrphans, divergent). New*Configbeans add a row here — no per-section parity test file.*Configbean iff the property's Java type has a default constructor, lives inorg.tron.*, and the HOCON value is an OBJECT. Lists / primitives / JDK types never recurse.[parity-hocon],[parity-bean],[parity-default],[parity-sweep]per section, then a final[parity-summary]block with the file-coverage equationfile-hoconKey = checkSection + cantCheckSectionand the bean-coverage triple-equalityregistry = parity-bean = parity-default. When the gate runs without booting a tron main, the lines land incommon/build/test-results/test/TEST-...ConfigParityGateTest.xmlunder<system-out>; once a main has touched logback in the same fork, they route tologs/on disk — class Javadoc spells this out.Allowlists preserved (do not extend):
committee.{allowPBFT,pBFTExpireNum},node.{isOpenFullTcpDisconnect,metrics},event.subscribe.{native,topics}.committee.{allowPbft,pbftExpireNum},node.openFullTcpDisconnect.genesis.block.{timestamp,parentHash,assets,witnesses},node.fastForward,event.subscribe.filter.{contractAddress,contractTopic}.