CARDS-1997: cards-data-model-forms-impl tests - #1266
Open
sofi2002sofi wants to merge 52 commits into
Open
Conversation
Contributor
|
Rebased on dev, compilations errors
|
sofi2002sofi
force-pushed
the
CARDS-1997-forms-tests
branch
from
July 2, 2023 09:08
590f387 to
74bcca0
Compare
veronikaslc
force-pushed
the
CARDS-1997-forms-tests
branch
from
November 20, 2025 19:21
8e7773a to
e5554b4
Compare
veronikaslc
force-pushed
the
CARDS-1997-forms-tests
branch
7 times, most recently
from
December 6, 2025 06:31
1ba55f9 to
1f5d2fd
Compare
sdumitriu
force-pushed
the
CARDS-1997-forms-tests
branch
from
February 9, 2026 21:27
88ae8d7 to
f887bcd
Compare
marta-
force-pushed
the
CARDS-1997-forms-tests
branch
from
June 23, 2026 23:22
91ae907 to
90157bc
Compare
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
|
||
| @SuppressWarnings("unchecked") | ||
| @Test | ||
| public void leaveForRootSubjectNode() throws RepositoryException, NoSuchFieldException, IllegalAccessException |
Contributor
|
Rebased onto the latest dev. This branch was far behind dev, so the tests needed some catching up:
All tests pass now (forms 354, pedigree 9, dicom 11, vocabularies 50). Running the tests (skipped by default, so add
The state of the branch before rebase is captured in |
sdumitriu
force-pushed
the
CARDS-1997-forms-tests
branch
from
July 19, 2026 01:39
9331fd2 to
a6a04a3
Compare
sdumitriu
force-pushed
the
CARDS-1997-forms-tests
branch
from
August 14, 2026 21:58
697e318 to
1d04b3c
Compare
tests for FormUtilImpl
change required coverage percent to enable project build
tests for QuestionnaireUtilsImpl
tests for ExpressionUtilsImpl
tests for ComputedAnswersEditor
tests for FormRelatedSubjectsEditor
tests for FormToMarkdownProcessor and FormToTextAdapterFactory
tests for QuestionnaireToCsvProcessor
Fixed failing tests
Set the current coverage percentage
Add more tests
After rebasing onto dev, the test suite no longer compiled because dev
moved several APIs forward since this branch was written. This commit
updates the in-module tests to match current dev. No production code is
changed.
Changes:
- Mockito 1.10 -> 5.23: the old `org.mockito.runners.MockitoJUnitRunner`
and `org.mockito.Matchers` were removed in newer Mockito. Switched to
`org.mockito.junit.MockitoJUnitRunner` and `org.mockito.ArgumentMatchers`.
- JSON API: dev moved from `javax.json` to `jakarta.json`. Updated all
test imports and types accordingly.
- Sling servlet API: dev moved to the Jakarta servlet API. Ported
QuestionnaireCSVServletTest to build requests/responses through the
public `Builders` API (`buildJakartaRequest` / `buildJakartaResponseResult`).
- Label processors: dev moved the base label processors to the exported
package `io.uhndata.cards.forms.serialize.labels`. Moved the seven tests
whose classes still live in this module into that package.
- Dropped the test-scoped Guava dependency, because dev removed Guava from
the build entirely; the tests do not use it directly.
- Removed the module-level `-Werror` / `failOnWarning` compiler block this
branch had added, so the module follows dev's shared compiler settings.
Added `@SuppressWarnings("unchecked")` to the processor and label tests
for the unavoidable raw-type mocks, matching the existing convention in
DataProcessorTest.
- Set the three label tests with intentionally lenient stubs to use
`MockitoJUnitRunner.Silent`, since Mockito 5 runs strict by default.
Note: some tests still fail at runtime (JCR node-type setup, expression
evaluation, and a few others) and are addressed separately.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Follow-up to the previous commit, fixing runtime test failures that only showed up once the suite compiled and ran against current dev. - Node types: the FormToMarkdown, FormToText and QuestionnaireToCsv serialization tests build an answer node of type cards:PedigreeAnswer. That node type is now defined in the pedigree module, which cannot be a dependency of this module (it depends on this module instead), so the test repository did not know the type and node creation failed. The serializers here still handle cards:PedigreeAnswer by node type, so the coverage belongs here. Added a test-only copy of the node type (PedigreeAnswer.cnd) and register it in each test's setup through Jackrabbit's CndImporter. - Strict stubbing: Mockito 5 runs stubs strictly by default, so tests that were written for the older lenient Mockito failed with UnnecessaryStubbingException. Set the affected tests to use MockitoJUnitRunner.Silent so they keep their original behaviour. With these changes the module builds and 348 of 354 tests pass. The remaining six failures are in ExpressionUtilsImplTest, where the mocked script engine is stubbed for the old single-line script; dev now builds a multi-line script and fills the bindings differently. Those are handled separately. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Dev moved four specialized label processors out of forms-impl into their
own question-type modules. Their tests were still in forms-impl, where the
processor classes no longer exist, so they could not compile there. This
moves each test to the module that now owns its processor:
- PedigreeLabelProcessorTest -> questionTypes/pedigree
- DicomLabelProcessorTest -> questionTypes/dicom
- VocabularyLabelProcessorTest, VocabularyOptionsLabelProcessorTest
-> vocabularies
To support these tests:
- Added the test dependencies (sling-mock, jcr-mock, JUnit, Mockito) to the
pedigree and dicom modules, which had no tests before. The vocabularies
module already had them.
- Copied the shared Questionnaires.json and SubjectTypes.json fixtures the
tests load into each module's test resources.
- Carried over the same API updates made elsewhere (Mockito 5 imports,
jakarta.json, @SuppressWarnings("unchecked") for the raw mocks, and the
Silent runner for VocabularyOptionsLabelProcessorTest).
All four tests now pass in their new modules (pedigree 9, dicom 11,
vocabularies 50 including the two moved tests).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
These tests mock the JavaScript engine and stub engine.eval(...) with the
exact script the evaluator used to build: "(function(){<expr>})()". Dev now
wraps the expression in a larger script (it declares each argument, assigns
"let result = (function(){<expr>})();" and returns "result;"), so the old
exact-match stubs never matched and evaluate() returned null.
The generated script still contains the "(function(){<expr>})()" fragment, so
the stubs now match on that fragment with Mockito.contains instead of
Mockito.eq. The bindings argument is unchanged: the test holds the same
Bindings instance the mocked createBindings() returns, so eq still matches.
Also set the class to MockitoJUnitRunner.Silent, since one test intentionally
keeps stubs that do not fire and Mockito 5 is strict by default.
The whole module now passes: 354 tests, 0 failures, 0 errors.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Cleanups so the full build (not just the test run) passes: - Import order: the javax -> jakarta JSON change left the jakarta imports in the same block as the javax ones. Checkstyle wants each import group separated by a blank line, so added the separator in the affected tests. - Line length: wrapped four lines in ExpressionUtilsImplTest that went over 120 characters after switching to Mockito.contains. - Coverage: lowered the module's minimum instruction coverage from 0.67 to 0.65. Four label-processor tests moved to other modules, and they had also exercised the shared base classes here, so this module's measured coverage dropped slightly. 0.65 reflects the current real number. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Follow-up to moving the label-processor tests into their question-type modules, so the full build passes there too: - Declared the test dependencies the moved tests actually use but that were only coming in transitively: org.apache.sling.api and org.apache.sling.resourcebuilder (both test scope). The dependency check flagged these as used-but-undeclared. The pedigree and dicom modules needed both; vocabularies only needed resourcebuilder. - Import order: added the blank line separating the jakarta imports from the javax ones, the same checkstyle fix applied in forms-impl. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Bump cards-data-model-forms-impl test dependencies (sling-mock core/junit4 4.0.6, sling-mock-oak 4.1.0-1.86.0, jcr-mock 1.8.2) so they match the dicom/pedigree/vocabularies modules and the rest of the repo, which had been left on the pre-rebase 3.5.6/3.2.0/1.6.14 versions.
…s/pom.xml Apache license header
…istic The three serialize tests compared the serialized creation-date line against java.time.LocalDate.now(), which is in the JVM default time zone. The serializer instead prints the date part of the form's jcr:created property, so the assertion could fail across a midnight boundary or when the stored timezone differs from the default one. Derive the expected date from the form node's actual jcr:created value, formatted in that property's own timezone, mirroring AbstractFormToStringSerializer.
Replace the mock(DataFilters.class) with full-method stubbing in leaveForRootSubjectNodeWithFormSelectorsOption with an anonymous concrete DataFilters implementation returning the same values. Clears the CodeQL "mocking all public methods" warning without changing behavior or assertions.
Remove the unused generateFilters helper from DataProcessorTest
Tighten the serialize tests - Use LinkedHashMap in the Resource->JsonObject test adapters so the JSON entry order matches the real declared resource order, instead of HashMap iteration order which encoded an accidental footer-before-body section ordering and hash-order CSV columns - Update the expected serializations accordingly - Expose the parent subject type under 'parents' in the CSV test adapter, like the real serializer does, covering gatherSubjectTypes' recursion, and assert the exact required-subject-type header row - Assert the array-vs-object distinction between recurrent and non-recurrent sections in BareFormProcessorTest - Verify the exact configuration path looked up by AnswerCopyProcessor.start - Pin pass-through processChild contracts with assertSame - Rename tests whose names contradicted what they assert, use a subject as the source of subject answer copying, and rename the leftover 'markdown' variable in the plain text serializer test
Fix vacuously-passing label processor tests Six mocked exception tests stubbed isNodeType with the concrete answer type while production checks the base cards:Answer type, so leave() exited before reaching any stubbed exception and the tests passed without exercising anything. Stub the type production actually checks, drop the MockitoJUnitRunner.Silent runners that were hiding the dead stubs, delete stubs of methods production never calls, remove dead copy-pasted fixture nodes, and rename tests whose names described a different scenario than the one they exercise.
Close coverage gaps in the label processor tests - Test the built-in Yes/No default labels and the -1 unknown value in BooleanLabelProcessorTest, and point its question constant at the actual boolean question instead of a computed one - Test the default date format and the missing-question guard in DateLabelProcessorTest - Use two distinct file names in the multivalued file label test so the value-to-label mapping is actually asserted
Add tests for the range label processors
Fix no-op tests and over-mocking in the editor and validator tests - ExpressionUtilsImplTest's exception test stubbed eval() with an eq() matcher on a script shape the current evaluator no longer produces, so it passed without exercising anything; split it into real formatter- failure and failing-script tests using contains(), and assert the parsed default values through the actual bindings instead of comparing a bindings instance to itself - ComputedAnswersEditorTest.handleLeaveTest asserted nothing and never even reached computeAnswer; it now verifies the evaluated result is written into the computed answer node - Seed SubjectHierarchyValidatorTest through public enter()/leave() calls instead of reflection, and stub isSubjectType per-argument so the hierarchy walk terminates through its loop condition rather than a silently-swallowed exception - Assert real match metadata in FormsQuickSearchEngineTest and return actual form ancestors instead of answer nodes from the getForm stub - Pre-set a value in handleLeaveForUncompletedSourceAnswer so removing it is actually observable - Replace call-order-coupled consecutive stubbing with per-argument stubbing in the editor tests - Switch all Silent Mockito runners to strict and delete the dead stubs they were hiding - Delete the constructorTest no-ops asserting on @Before-assigned fields, trim fixture subtrees the classes under test never read, make the findAll*RelatedAnswers assertions independent of getReferences() iteration order, and fix misleading test names and typos
Register each module's node types explicitly in its tests
Updated the coverage threshold
…-typed objects for Date-typed answers instead of Date-typed objects Cover the Oak date conversion with tests
sdumitriu
force-pushed
the
CARDS-1997-forms-tests
branch
from
August 14, 2026 21:58
1d04b3c to
46176f2
Compare
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
4 participants
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.
No description provided.