Migrate tests to use IModuleBuilder instead of ModuleLoader #531
Conversation
Replace ModuleLoader with IModuleBuilder for programmatic module construction. The test still loads external constraints from XML (XmlMetaConstraintLoader) but the module itself is now built using the mock builder infrastructure.
Replace ModuleLoader with IModuleBuilder for programmatic module construction in both RecursionCollectingNodeItemVisitorTest and AbstractRecursionPreventingNodeItemVisitorTest. These tests now use a simple self-referencing assembly structure instead of loading the complex metaschema-module-metaschema.xml file. The tests verify: - RecursionCollectingNodeItemVisitor correctly detects recursive assemblies - AbstractRecursionPreventingNodeItemVisitor handles recursion without infinite loops
Replace ModuleLoader with IModuleBuilder for programmatic module construction. The test now uses a simple module structure with a root assembly containing fields and nested assemblies, instead of loading the complex OSCAL metaschema from GitHub. The test verifies that MermaidErDiagramGenerator produces valid output containing expected elements.
📝 WalkthroughWalkthroughTests were refactored to build metaschema modules in-memory using IModuleBuilder and MockedModelTestSupport instead of loading XML from disk; many test-support classes were moved to new packages and imports updated accordingly. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
core/src/test/java/gov/nist/secauto/metaschema/core/metapath/item/node/AbstractRecursionPreventingNodeItemVisitorTest.java (1)
20-38: Strengthen recursion test with an explicit timeout/assertionThe in-memory module with a self-referencing
recursive-nodeassembly is a clean way to exercise recursion prevention, and the visitor setup itself looks correct. Right now, the test passes implicitly by “not hanging / not overflowing”. To make the intent explicit and avoid a stuck build if recursion prevention regresses, consider wrapping the visit in a JUnit 5 timeout assertion (and optionally asserting no exception), e.g.:// example sketch assertTimeoutPreemptively(Duration.ofSeconds(1), () -> { visitor.visitMetaschema( INodeItemFactory.instance().newModuleNodeItem(module), null); });This would clearly document that the expectation is “completes quickly without infinite recursion” and guard against future regressions.
Also applies to: 48-50
core/src/test/java/gov/nist/secauto/metaschema/core/util/MermaidErDiagramGeneratorTest.java (1)
28-64: Excellent migration to programmatic module construction!The refactoring successfully eliminates external file dependencies and exception handling overhead. The builder pattern is used correctly, and resource management is proper with try-with-resources.
Optional: Consider enhancing test assertions.
The test creates
child-item,title, anddescriptionelements but only verifiesrootappears in the diagram. Consider adding assertions to ensure all created elements are rendered:String diagram = stringWriter.toString(); // Verify the diagram contains expected elements assertTrue(diagram.contains("erDiagram"), "Diagram should start with erDiagram"); assertTrue(diagram.contains("root"), "Diagram should contain root assembly"); +assertTrue(diagram.contains("child-item"), "Diagram should contain child-item assembly"); +assertTrue(diagram.contains("title"), "Diagram should contain title field"); +assertTrue(diagram.contains("description"), "Diagram should contain description field");This would provide more comprehensive verification of the diagram generation, though the current test adequately validates the core functionality.
core/src/test/java/gov/nist/secauto/metaschema/core/model/constraint/ExternalConstraintsModulePostProcessorTest.java (1)
33-62: Programmatic module construction and constraint verification look correctBuilding the module via
IModuleBuilder/MockedModelTestSupportwith assembly"a"(including the"value"flag and selfassemblyRef("a")) and then applyingExternalConstraintsModulePostProcessorbefore asserting a single constraint on that assembly preserves the original test intent while removing XML-based module loading. UsingTEST_NAMESPACEconsistently in both the builder andIEnhancedQName.of(TEST_NAMESPACE, "a")avoids magic strings and keeps the constraints and module aligned.If you later find this pattern repeating, consider extracting small helpers/constants for the assembly name
"a"and flag name"value"so the module definition and QNames stay trivially in sync across tests.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
core/src/test/java/gov/nist/secauto/metaschema/core/metapath/item/node/AbstractRecursionPreventingNodeItemVisitorTest.java(2 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/metapath/item/node/RecursionCollectingNodeItemVisitorTest.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/model/constraint/ExternalConstraintsModulePostProcessorTest.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/util/MermaidErDiagramGeneratorTest.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
core/src/test/java/gov/nist/secauto/metaschema/core/util/MermaidErDiagramGeneratorTest.java (1)
core/src/test/java/gov/nist/secauto/metaschema/core/testing/model/MockedModelTestSupport.java (1)
MockedModelTestSupport(14-24)
core/src/test/java/gov/nist/secauto/metaschema/core/metapath/item/node/AbstractRecursionPreventingNodeItemVisitorTest.java (1)
core/src/test/java/gov/nist/secauto/metaschema/core/testing/model/MockedModelTestSupport.java (1)
MockedModelTestSupport(14-24)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Code
- GitHub Check: Website
🔇 Additional comments (5)
core/src/test/java/gov/nist/secauto/metaschema/core/metapath/item/node/RecursionCollectingNodeItemVisitorTest.java (1)
1-56: Excellent migration to programmatic module construction.The refactoring cleanly replaces file-based XML loading with in-memory programmatic module construction. The test is now:
- More readable and focused on the specific behavior being tested
- Faster (no I/O overhead)
- Self-contained with explicit test data
- Consistent with the migration pattern adopted in other test classes (ExternalConstraintsModulePostProcessorTest, etc.)
The assertions are well-structured: you verify detection, cardinality, and the specific assembly name. The iterator safety is properly guarded by the prior isEmpty() check.
core/src/test/java/gov/nist/secauto/metaschema/core/metapath/item/node/AbstractRecursionPreventingNodeItemVisitorTest.java (1)
8-16: Imports and test utilities align with new module-builder test patternSwitching to
ISource,IModuleBuilder, andMockedModelTestSupportplusURI/Listis consistent with the PR objective of programmatic module construction and avoids file-based loading. No issues here; this looks like the right direction.core/src/test/java/gov/nist/secauto/metaschema/core/util/MermaidErDiagramGeneratorTest.java (2)
8-21: LGTM! Import changes align with the migration.The updated imports correctly support the shift from file-based module loading to programmatic construction using
IModuleBuilderandMockedModelTestSupport.
25-25: LGTM! Clear constant definition.Extracting the namespace as a constant improves maintainability.
core/src/test/java/gov/nist/secauto/metaschema/core/model/constraint/ExternalConstraintsModulePostProcessorTest.java (1)
12-23: Namespace/source wiring and imports align with the new in-memory approachUsing
TEST_NAMESPACEtogether withISource.externalSource(URI.create(TEST_NAMESPACE))and the added testing-model imports cleanly supports programmatic module construction while keeping the namespace defined in a single place. No issues here.Also applies to: 29-29
Rename and restructure the test support infrastructure from `core.testing.model` to `core.testsupport` with a clearer layout: - testsupport/ - MockedModelTestSupport.java (main entry point) - mocking/ (low-level mock factories) - builder/ (fluent builders for test fixtures) - tests/ (tests for the support infrastructure) This makes the purpose of these classes clearer - they are test support utilities, not tests themselves.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/mocking/IMockFactory.java (1)
6-6: Package relocation is appropriate.The simplified package structure (
testsupport.mockingvs.testing.model.mocking) improves clarity and aligns with the PR's test infrastructure restructuring objectives.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (48)
core/metaschema(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/metapath/cst/AnonymousFunctionCallTest.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/metapath/cst/BuildCstVisitorTest.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/metapath/cst/logic/PredicateExpressionTest.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/metapath/cst/path/RootSlashOnlyPathTest.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/metapath/cst/path/StepTest.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/metapath/cst/type/InstanceOfTest.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnBaseUriTest.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnDataTest.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnDocumentUriTest.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnHasChildrenTest.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnInnermostTest.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnLocalNameTest.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnNameTest.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnNamespaceUriTest.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnOutermostTest.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnPathTest.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnRootTest.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/metapath/item/node/AbstractRecursionPreventingNodeItemVisitorTest.java(2 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/metapath/item/node/DefaultNodeItemFactoryTest.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/metapath/item/node/RecursionCollectingNodeItemVisitorTest.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/model/constraint/DefaultConstraintValidatorTest.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/model/constraint/ExternalConstraintsModulePostProcessorTest.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/model/constraint/impl/AbstractConfigurableMessageConstraintTest.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/MockedModelTestSupport.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/builder/AbstractMetaschemaBuilder.java(2 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/builder/AbstractModelBuilder.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/builder/AssemblyBuilder.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/builder/AssemblyReference.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/builder/FieldBuilder.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/builder/FieldReference.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/builder/FlagBuilder.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/builder/IAssemblyBuilder.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/builder/IFieldBuilder.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/builder/IFlagBuilder.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/builder/IMetaschemaBuilder.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/builder/IModelBuilder.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/builder/IModelReference.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/builder/IModuleBuilder.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/builder/IModuleMockFactory.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/builder/ModuleBuilder.java(2 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/mocking/AbstractJMockFactory.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/mocking/AbstractMockitoFactory.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/mocking/IMockFactory.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/mocking/MockNodeItemFactory.java(2 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/mocking/MockedDocumentGenerator.java(2 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/tests/ModuleBuilderTest.java(2 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/util/MermaidErDiagramGeneratorTest.java(1 hunks)
✅ Files skipped from review due to trivial changes (11)
- core/metaschema
- core/src/test/java/gov/nist/secauto/metaschema/core/model/constraint/impl/AbstractConfigurableMessageConstraintTest.java
- core/src/test/java/gov/nist/secauto/metaschema/core/metapath/cst/AnonymousFunctionCallTest.java
- core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnBaseUriTest.java
- core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/builder/IModuleMockFactory.java
- core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/builder/AbstractModelBuilder.java
- core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/builder/AssemblyReference.java
- core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnDataTest.java
- core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnOutermostTest.java
- core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/builder/IFlagBuilder.java
- core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/builder/FieldBuilder.java
🚧 Files skipped from review as they are similar to previous changes (2)
- core/src/test/java/gov/nist/secauto/metaschema/core/model/constraint/ExternalConstraintsModulePostProcessorTest.java
- core/src/test/java/gov/nist/secauto/metaschema/core/metapath/item/node/RecursionCollectingNodeItemVisitorTest.java
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2024-11-14T18:19:40.200Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 245
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/IBooleanItem.java:86-104
Timestamp: 2024-11-14T18:19:40.200Z
Learning: In the file `core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/IBooleanItem.java`, the 3-step approach in the `cast` method is consistent with the XPath 3.1 specification.
Applied to files:
core/src/test/java/gov/nist/secauto/metaschema/core/metapath/cst/path/StepTest.javacore/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnHasChildrenTest.javacore/src/test/java/gov/nist/secauto/metaschema/core/metapath/cst/type/InstanceOfTest.javacore/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnPathTest.javacore/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnLocalNameTest.javacore/src/test/java/gov/nist/secauto/metaschema/core/metapath/cst/BuildCstVisitorTest.javacore/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnDocumentUriTest.javacore/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnRootTest.javacore/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnNameTest.javacore/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnNamespaceUriTest.javacore/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnInnermostTest.javacore/src/test/java/gov/nist/secauto/metaschema/core/metapath/cst/logic/PredicateExpressionTest.javacore/src/test/java/gov/nist/secauto/metaschema/core/metapath/cst/path/RootSlashOnlyPathTest.java
📚 Learning: 2024-11-14T17:09:05.819Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 245
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/INonNegativeIntegerItem.java:116-124
Timestamp: 2024-11-14T17:09:05.819Z
Learning: In the interface `INonNegativeIntegerItem` (file `core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/INonNegativeIntegerItem.java`), the casting logic in the `cast` method is intentionally designed this way due to earlier discrepancies.
Applied to files:
core/src/test/java/gov/nist/secauto/metaschema/core/metapath/cst/path/StepTest.javacore/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnHasChildrenTest.javacore/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnLocalNameTest.javacore/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnRootTest.javacore/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnNameTest.javacore/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnNamespaceUriTest.javacore/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnInnermostTest.java
📚 Learning: 2024-11-28T04:53:23.842Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 266
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/DynamicFunctionCall.java:73-77
Timestamp: 2024-11-28T04:53:23.842Z
Learning: In the `DynamicFunctionCall` class in `core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/DynamicFunctionCall.java`, it's acceptable to rely on `toAtomicItem()` to throw an exception if the specifier is not an `IAtomicItem`, rather than checking the instance type before calling `toAtomicItem()`.
Applied to files:
core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnHasChildrenTest.javacore/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnLocalNameTest.javacore/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnDocumentUriTest.javacore/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnRootTest.javacore/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnNameTest.javacore/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnNamespaceUriTest.java
📚 Learning: 2024-11-14T05:20:24.045Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 245
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/IYearMonthDurationItem.java:87-94
Timestamp: 2024-11-14T05:20:24.045Z
Learning: In the `IYearMonthDurationItem` interface (`core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/IYearMonthDurationItem.java`), the `cast` method uses `item.asString()` as intended. Do not suggest replacing it with `FunctionUtils.toString(item)` in this context.
Applied to files:
core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnHasChildrenTest.javacore/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnLocalNameTest.javacore/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnRootTest.javacore/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnNameTest.javacore/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnNamespaceUriTest.java
📚 Learning: 2024-11-14T17:07:03.586Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 245
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/IIPv4AddressItem.java:66-73
Timestamp: 2024-11-14T17:07:03.586Z
Learning: In the Metaschema Java codebase, differences in casting patterns across atomic type implementations are intentional and required; any differences in approach are significant and necessary.
Applied to files:
core/src/test/java/gov/nist/secauto/metaschema/core/metapath/cst/type/InstanceOfTest.java
📚 Learning: 2025-01-07T00:50:46.218Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 336
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/type/InstanceOf.java:87-88
Timestamp: 2025-01-07T00:50:46.218Z
Learning: In the Metapath implementation, child expressions should be called using `accept()` rather than `evaluate()` to ensure proper management of the execution stack through push/pop operations. The `evaluate()` method is an internal implementation detail that performs the actual evaluation.
Applied to files:
core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnPathTest.java
📚 Learning: 2025-01-07T00:51:17.257Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 336
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/items/ArraySequenceConstructor.java:63-63
Timestamp: 2025-01-07T00:51:17.257Z
Learning: In the Metapath implementation, expression evaluation must go through the `accept()` method rather than calling `evaluate()` directly, as `accept()` handles the push/pop operations on the execution stack which is used for debugging and error reporting.
Applied to files:
core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnPathTest.java
🧬 Code graph analysis (16)
core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/builder/ModuleBuilder.java (1)
core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/mocking/AbstractMockitoFactory.java (1)
AbstractMockitoFactory(24-55)
core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnHasChildrenTest.java (1)
core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/mocking/MockedDocumentGenerator.java (1)
MockedDocumentGenerator(32-150)
core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnPathTest.java (1)
core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/mocking/MockedDocumentGenerator.java (1)
MockedDocumentGenerator(32-150)
core/src/test/java/gov/nist/secauto/metaschema/core/metapath/item/node/DefaultNodeItemFactoryTest.java (1)
core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/MockedModelTestSupport.java (1)
MockedModelTestSupport(15-25)
core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnLocalNameTest.java (1)
core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/mocking/MockedDocumentGenerator.java (1)
MockedDocumentGenerator(32-150)
core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/tests/ModuleBuilderTest.java (1)
core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/MockedModelTestSupport.java (1)
MockedModelTestSupport(15-25)
core/src/test/java/gov/nist/secauto/metaschema/core/metapath/item/node/AbstractRecursionPreventingNodeItemVisitorTest.java (1)
core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/MockedModelTestSupport.java (1)
MockedModelTestSupport(15-25)
core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnDocumentUriTest.java (1)
core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/mocking/MockedDocumentGenerator.java (1)
MockedDocumentGenerator(32-150)
core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnRootTest.java (1)
core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/mocking/MockedDocumentGenerator.java (1)
MockedDocumentGenerator(32-150)
core/src/test/java/gov/nist/secauto/metaschema/core/util/MermaidErDiagramGeneratorTest.java (1)
core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/MockedModelTestSupport.java (1)
MockedModelTestSupport(15-25)
core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnNameTest.java (1)
core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/mocking/MockedDocumentGenerator.java (1)
MockedDocumentGenerator(32-150)
core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/builder/AbstractMetaschemaBuilder.java (1)
core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/mocking/AbstractMockitoFactory.java (1)
AbstractMockitoFactory(24-55)
core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/MockedModelTestSupport.java (1)
core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/mocking/AbstractMockitoFactory.java (1)
AbstractMockitoFactory(24-55)
core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnNamespaceUriTest.java (1)
core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/mocking/MockedDocumentGenerator.java (1)
MockedDocumentGenerator(32-150)
core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnInnermostTest.java (1)
core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/mocking/MockedDocumentGenerator.java (1)
MockedDocumentGenerator(32-150)
core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/mocking/MockedDocumentGenerator.java (1)
core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/MockedModelTestSupport.java (1)
MockedModelTestSupport(15-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Website
- GitHub Check: Code
🔇 Additional comments (40)
core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/builder/FlagBuilder.java (1)
6-6: Package relocation looks good.The package rename from
gov.nist.secauto.metaschema.core.testing.modeltogov.nist.secauto.metaschema.core.testsupport.builderaligns with the PR objectives to reorganize test support infrastructure. Since the full test suite passes, the migration appears complete. Verify that all references to the old package path have been updated across the codebase to ensure no stale imports remain.core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/builder/IFieldBuilder.java (1)
6-6: Package relocation looks good; verify consistency across codebase.The package move from
gov.nist.secauto.metaschema.core.testing.modeltogov.nist.secauto.metaschema.core.testsupport.builderaligns with the PR objectives to reorganize test support infrastructure.Ensure that all imports referencing the old package path have been updated, and verify that the
FieldBuilderclass (referenced in line 29) is accessible in the new package structure.core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnInnermostTest.java (1)
20-20: Import updated to new testsupport locationThe
MockedDocumentGeneratorimport now points atcore.testsupport.mocking, which matches the new test-support package layout. No behavioral change; looks correct.core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnLocalNameTest.java (1)
19-19: Consistent migration tocore.testsupport.mockingThe
MockedDocumentGeneratorimport correctly targets the newcore.testsupport.mockingpackage; existing usages (constants and generator) remain valid.core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnDocumentUriTest.java (1)
20-20: Updated document generator import is correctThe import now references
MockedDocumentGeneratorincore.testsupport.mocking, which aligns with its new location and preserves the existing test behavior.core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/mocking/AbstractJMockFactory.java (1)
6-6: Package relocation matches new testsupport namespace
AbstractJMockFactoryis now ingov.nist.secauto.metaschema.core.testsupport.mocking, matching the file path and the rest of the mocking test-support utilities.core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/builder/IModuleBuilder.java (1)
6-6: Builder interface correctly moved tocore.testsupport.builder
IModuleBuildernow resides in thecore.testsupport.builderpackage, consistent with the new test-support builder layout and without altering its API.core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/builder/FieldReference.java (1)
6-6: FieldReference package updated in line with builder refactorThe package is now
gov.nist.secauto.metaschema.core.testsupport.builder, which aligns with the rest of the in-memory module builder support classes.core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/builder/IAssemblyBuilder.java (1)
6-6: IAssemblyBuilder correctly repackaged under testsupport.builderThe interface now lives in
core.testsupport.builder, matching the new location ofAssemblyBuilderand related builders with no functional modification.core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/mocking/AbstractMockitoFactory.java (1)
6-6: Mockito-based mock factory moved to testsupport.mocking
AbstractMockitoFactoryis now undergov.nist.secauto.metaschema.core.testsupport.mocking, consistent with the rest of the mocking test-support classes; behavior remains the same.core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/builder/AbstractMetaschemaBuilder.java (1)
6-6: LGTM - Clean package refactoring.The package relocation from
core.testing.modeltocore.testsupport.builderand corresponding import update forAbstractMockitoFactoryare consistent with the broader test support reorganization in this PR.Also applies to: 19-19
core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/builder/AssemblyBuilder.java (1)
6-6: LGTM - Package relocation.The package change to
core.testsupport.builderis consistent with the test support infrastructure reorganization.core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/builder/IModelReference.java (1)
6-6: LGTM - Package relocation.The package change to
core.testsupport.builderaligns with the broader test support reorganization.core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnPathTest.java (1)
19-19: LGTM - Import path update.The import for
MockedDocumentGeneratorcorrectly reflects its relocation tocore.testsupport.mocking.core/src/test/java/gov/nist/secauto/metaschema/core/metapath/cst/path/RootSlashOnlyPathTest.java (1)
19-19: LGTM - Import path update.The import for
MockNodeItemFactorycorrectly reflects its relocation tocore.testsupport.mocking.core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/tests/ModuleBuilderTest.java (1)
6-6: LGTM - Package reorganization and import additions.The package change to
core.testsupport.testsand the added imports forMockedModelTestSupportandIModuleBuilderare consistent with the test support infrastructure migration described in the PR objectives.Also applies to: 19-20
core/src/test/java/gov/nist/secauto/metaschema/core/metapath/item/node/DefaultNodeItemFactoryTest.java (1)
13-13: LGTM - Import path update.The import for
MockedModelTestSupportcorrectly reflects its relocation tocore.testsupport.core/src/test/java/gov/nist/secauto/metaschema/core/metapath/cst/type/InstanceOfTest.java (1)
22-22: LGTM - Import path update.The import for
MockNodeItemFactorycorrectly reflects its relocation tocore.testsupport.mocking.core/src/test/java/gov/nist/secauto/metaschema/core/metapath/cst/logic/PredicateExpressionTest.java (1)
20-20: LGTM! Clean import path update.The import path update for
MockNodeItemFactoryaligns with the test support package reorganization fromcore.testing.model.mockingtocore.testsupport.mocking.core/src/test/java/gov/nist/secauto/metaschema/core/model/constraint/DefaultConstraintValidatorTest.java (1)
31-31: LGTM! Import path updated correctly.The
MockNodeItemFactoryimport path has been correctly updated to reflect the test support package reorganization.core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnNameTest.java (1)
19-19: LGTM! Import path relocated correctly.The
MockedDocumentGeneratorimport has been updated to the newtestsupport.mockingpackage, consistent with the broader test infrastructure reorganization.core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnHasChildrenTest.java (1)
18-18: LGTM! Test support import updated correctly.The import path for
MockedDocumentGeneratorhas been correctly updated to reflect the new test support package structure.core/src/test/java/gov/nist/secauto/metaschema/core/metapath/cst/path/StepTest.java (1)
20-20: LGTM! Import path correctly updated.The
MockNodeItemFactoryimport path has been updated to align with the test support package reorganization tocore.testsupport.mocking.core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnRootTest.java (1)
18-18: LGTM! Import relocated correctly.The import path for
MockedDocumentGeneratorhas been updated to the new test support package location.core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnNamespaceUriTest.java (1)
19-19: LGTM! Test support import updated.The
MockedDocumentGeneratorimport path has been correctly updated to the newtestsupport.mockingpackage.core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/builder/ModuleBuilder.java (1)
6-6: LGTM! Package reorganization applied correctly.Both the package declaration and the
AbstractMockitoFactoryimport have been correctly updated to reflect the test support package reorganization fromcore.testing.modeltocore.testsupport.builderandcore.testsupport.mockingrespectively.Also applies to: 17-17
core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/mocking/MockNodeItemFactory.java (2)
6-6: LGTM: Package relocation aligns with test infrastructure reorganization.The package relocation from
gov.nist.secauto.metaschema.core.testing.model.mockingtogov.nist.secauto.metaschema.core.testsupport.mockingis part of the broader test support infrastructure reorganization documented in this PR.
41-41: LGTM: Comment updated to reflect new package structure.The FIXME comment correctly references the new
testsupportpackage namespace.core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/MockedModelTestSupport.java (1)
6-9: LGTM: Package relocation with consistent import updates.The package relocation and corresponding import path updates are consistent with the broader test support infrastructure reorganization.
core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/builder/IModelBuilder.java (1)
6-6: LGTM: Package relocation to builder subpackage.The relocation to
gov.nist.secauto.metaschema.core.testsupport.builderappropriately organizes builder interfaces under the test support namespace.core/src/test/java/gov/nist/secauto/metaschema/core/metapath/cst/BuildCstVisitorTest.java (1)
54-54: LGTM: Import path updated to reflect package relocation.The import update aligns with the MockNodeItemFactory package relocation to the testsupport.mocking namespace.
core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/builder/IMetaschemaBuilder.java (1)
6-6: LGTM: Package relocation to builder subpackage.Consistent with the broader reorganization of builder interfaces under
gov.nist.secauto.metaschema.core.testsupport.builder.core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/mocking/MockedDocumentGenerator.java (2)
6-6: LGTM: Package relocation to mocking subpackage.The relocation to
gov.nist.secauto.metaschema.core.testsupport.mockingappropriately organizes mocking utilities under the test support namespace.
21-21: LGTM: Import updated for relocated MockedModelTestSupport.The import path correctly reflects the new location of MockedModelTestSupport in the testsupport package.
core/src/test/java/gov/nist/secauto/metaschema/core/metapath/item/node/AbstractRecursionPreventingNodeItemVisitorTest.java (3)
1-4: LGTM: License headers added.The SPDX license headers are correctly formatted.
9-16: LGTM: Imports updated for in-memory module construction.The imports correctly reflect the migration from file-based module loading to in-memory construction using IModuleBuilder and MockedModelTestSupport.
20-50: LGTM: Test successfully migrated to in-memory module construction.The refactored test achieves the same goal of testing recursion prevention while eliminating file I/O dependencies. The self-referencing assembly structure (Line 37:
assemblyRef("recursive-node")) correctly creates the recursive scenario needed to verify the visitor's recursion prevention logic.Benefits of this approach:
- Removes external file dependencies
- Improves test isolation and reliability
- Eliminates IOException and MetaschemaException from test signature
- Makes the recursive structure explicit in the test code
core/src/test/java/gov/nist/secauto/metaschema/core/util/MermaidErDiagramGeneratorTest.java (3)
8-8: LGTM: Static import added for assertion.The static import for
assertTrueis appropriate for the assertion-based validation approach.
11-21: LGTM: Imports updated for in-memory module construction.The imports correctly support the migration from file-based module loading to in-memory construction using IModuleBuilder and MockedModelTestSupport.
25-64: LGTM: Test successfully migrated to in-memory module construction.The refactored test achieves the same goal of validating Mermaid ER diagram generation while eliminating external file dependencies. The in-memory module construction provides a clear, self-contained test structure with appropriate assertions.
Benefits of this approach:
- Removes dependency on external module files
- Improves test isolation and maintainability
- Uses StringWriter for output capture, avoiding file I/O
- Makes the module structure explicit in the test code
- Eliminates IOException, MetaschemaException, and URISyntaxException from test signature
- Add assertTimeoutPreemptively to AbstractRecursionPreventingNodeItemVisitorTest to make the recursion prevention intent explicit - Enhance MermaidErDiagramGeneratorTest with additional assertions for child-item, title, and description elements
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
core/src/test/java/gov/nist/secauto/metaschema/core/util/MermaidErDiagramGeneratorTest.java (1)
62-66: Consider more specific assertions for diagram content.The
contains()checks work but could potentially match unintended content (e.g., error messages, comments). For more robust verification, you might consider matching against expected Mermaid ER diagram syntax patterns.For example, you could verify the relationship syntax:
assertTrue(diagram.contains("erDiagram"), "Diagram should start with erDiagram"); assertTrue(diagram.contains("root"), "Diagram should contain root assembly"); assertTrue(diagram.contains("child-item"), "Diagram should contain child-item assembly"); - assertTrue(diagram.contains("title"), "Diagram should contain title field"); - assertTrue(diagram.contains("description"), "Diagram should contain description field"); + // Verify fields appear in relation to their parent assembly + assertTrue(diagram.contains("title") && diagram.contains("root"), + "Diagram should contain title field related to root"); + assertTrue(diagram.contains("description") && diagram.contains("root"), + "Diagram should contain description field related to root");That said, the current approach is acceptable for validating the basic functionality during this migration.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
core/src/test/java/gov/nist/secauto/metaschema/core/metapath/item/node/AbstractRecursionPreventingNodeItemVisitorTest.java(2 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/util/MermaidErDiagramGeneratorTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- core/src/test/java/gov/nist/secauto/metaschema/core/metapath/item/node/AbstractRecursionPreventingNodeItemVisitorTest.java
🧰 Additional context used
🧬 Code graph analysis (1)
core/src/test/java/gov/nist/secauto/metaschema/core/util/MermaidErDiagramGeneratorTest.java (1)
core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/MockedModelTestSupport.java (1)
MockedModelTestSupport(15-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Code
- GitHub Check: Website
🔇 Additional comments (1)
core/src/test/java/gov/nist/secauto/metaschema/core/util/MermaidErDiagramGeneratorTest.java (1)
28-67: LGTM! Clean migration to in-memory module construction.The test has been well-refactored to use
IModuleBuilderandMockedModelTestSupportfor programmatic module construction, eliminating the need for external file loading. The structure creates a representative module hierarchy (root assembly with fields and nested assemblies) that exercises the diagram generator appropriately.The use of
StringWriterfor capturing output and the content-based assertions provide good verification that the diagram generator produces expected output.
david-waltermire
merged commit eb67ca8
into
metaschema-framework:develop
1 participant
Summary
ExternalConstraintsModulePostProcessorTestto useIModuleBuilderfor programmatic module constructionRecursionCollectingNodeItemVisitorTestandAbstractRecursionPreventingNodeItemVisitorTestto useIModuleBuilderMermaidErDiagramGeneratorTestto useIModuleBuilderThis PR continues the work from #530 by migrating test classes to use the new module builder infrastructure, reducing dependencies on
ModuleLoaderand XMLBeans for loading test modules.Test plan
mvn -pl core test -Dtest=ExternalConstraintsModulePostProcessorTestpassesmvn -pl core test -Dtest=RecursionCollectingNodeItemVisitorTestpassesmvn -pl core test -Dtest=AbstractRecursionPreventingNodeItemVisitorTestpassesmvn -pl core test -Dtest=MermaidErDiagramGeneratorTestpassesmvn -pl core testpasses (full core module test suite)Summary by CodeRabbit
Tests
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.