This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch feature/WW-5695-html5-constraint-validation in repository https://gitbox.apache.org/repos/asf/struts.git
commit 88162c384aab7a99f218a269e99b248ac218e16d Author: Lukasz Lenart <[email protected]> AuthorDate: Mon Aug 24 23:07:43 2026 +0200 WW-5695 docs(validation): make the plan's memoisation test actually test memoisation A task reviewer found the plan's own testRepeatedCallsAreConsistent vacuous: resolution is deterministic, so comparing result sizes across two calls passes identically against an implementation with no cache at all. Memoisation is the entire purpose of getFieldValidators, so nothing would have caught it silently regressing to a full resolution per field. Replaced with a test that counts resolutions across two different field names, and recorded the second half of the harness trap: createMocks() never sets a config on the MockActionProxy, so the validator manager NPEs without an explicit setConfig. Co-Authored-By: Claude Opus 5 <[email protected]> --- .../2026-08-24-html5-constraint-validation.md | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/docs/superpowers/plans/2026-08-24-html5-constraint-validation.md b/docs/superpowers/plans/2026-08-24-html5-constraint-validation.md index f174acaa5..50a6a5668 100644 --- a/docs/superpowers/plans/2026-08-24-html5-constraint-validation.md +++ b/docs/superpowers/plans/2026-08-24-html5-constraint-validation.md @@ -950,11 +950,17 @@ public class FormFieldValidatorsTest extends AbstractUITagTest { assertTrue(form.getFieldValidators("noSuchField").isEmpty()); } - public void testRepeatedCallsAreConsistent() throws Exception { + public void testResolvesTheActionsValidatorsOnlyOnceAcrossFields() throws Exception { Form form = formForDoubleValidationAction(); - assertEquals(form.getFieldValidators("myUpDownSelectTag").size(), - form.getFieldValidators("myUpDownSelectTag").size()); + ActionValidatorManager manager = mock(ActionValidatorManager.class); + when(manager.getValidators(any(), any(), any())).thenReturn(Collections.emptyList()); + form.setActionValidatorManager(manager); + + form.getFieldValidators("myUpDownSelectTag"); + form.getFieldValidators("someOtherField"); + + verify(manager, times(1)).getValidators(any(), any(), any()); } private Form formForDoubleValidationAction() throws Exception { @@ -978,6 +984,16 @@ public class FormFieldValidatorsTest extends AbstractUITagTest { } ``` +**Harness trap, part two:** `createMocks()` never calls `setConfig` on the `MockActionProxy` it builds, so +`AnnotationActionValidatorManager.buildValidatorKey` dereferences a null `ActionConfig` and NPEs. The test's +`setUp` also needs `((MockActionProxy) actionProxy).setConfig(configuration.getRuntimeConfiguration() +.getActionConfig("", "doubleValidationAction"))`. This is why `FormTagTest` carries its own +`prepareMockInvocation()` helper. + +**Do not write a memoisation test that only compares result sizes** — resolution is deterministic, so such a +test passes identically against an implementation with no cache at all. Assert the *number of resolutions* +with a mocked `ActionValidatorManager`, across two different field names. + **Harness trap:** without `initDispatcher(configProviders = TestConfigurationProvider)` *and* `createMocks()`, the action config is not present and validator resolution silently returns nothing — the test would pass or fail for entirely the wrong reason. `DoubleValidationAction-validation.xml` already exists at `core/src/test/resources/org/apache/struts2/views/jsp/ui/` and declares a `double` validator for `myUpDownSelectTag`. - [ ] **Step 2: Run the test to verify it fails**
