TriggerableDagTest.java - port relevance exclusion test…

- Test exercises what seems like various internals. Minimal effort has been made to satisfy those interfaces so they type check.

- Test will fail before reaching those interfaces regardless.

- Includes a supplemental test exercising some similar aspects of excluding non-relevant nodes

- Supplemental test fails, caught a bug!

packages/scenario/src/jr/Scenario.ts

@@ -21,12 +21,15 @@ import {
 	type PositionalEvents,
 } from './event/getPositionalEvents.ts';
 import { isQuestionEventOfType, type TypedQuestionEvent } from './event/predicates.ts';
+import { JRFormDef } from './form/JRFormDef.ts';
 import { TreeReference } from './instance/TreeReference.ts';
 import type { FormDefinitionResource } from './resource/FormDefinitionResource.ts';
 import { r } from './resource/ResourcePathHelper.ts';
 import { SelectChoiceList } from './select/SelectChoiceList.ts';
 import type { ValidateOutcome } from './validation/ValidateOutcome.ts';
 import { ValidationImplementationPendingError } from './validation/ValidationImplementationPendingError.ts';
+import { JREvaluationContext } from './xpath/JREvaluationContext.ts';
+import { JRTreeReference } from './xpath/JRTreeReference.ts';
 
 interface ScenarioConstructorOptions {
 	readonly dispose: VoidFunction;
@@ -546,4 +549,41 @@ export class Scenario {
 			}) ?? null
 		);
 	}
+
+	// TODO: consider adapting tests which use the following interfaces to use
+	// more portable concepts (either by using conceptually similar `Scenario`
+	// APIs, or by reframing the tests' logic to the same behavioral concerns with
+	// better supported APIs)
+
+	/**
+	 * @todo Mark deprecated?
+	 */
+	getFormDef(): JRFormDef {
+		return new JRFormDef(this.instanceRoot);
+	}
+
+	/**
+	 * @see {@link JREvaluationContext}
+	 * @todo Mark deprecated?
+	 */
+	getEvaluationContext(): JREvaluationContext {
+		return new JREvaluationContext();
+	}
+
+	/**
+	 * @todo Mark deprecated?
+	 */
+	expandSingle(_treeReference: JRTreeReference): JRTreeReference {
+		throw new UnclearApplicabilityError('XPath internals');
+	}
 }
+
+/**
+ * JavaRosa exposes this as a static method on {@link Scenario}, but we expose
+ * it as a named export as that is its typical usage in ported tests.
+ *
+ * @todo Mark deprecated?
+ */
+export const getRef = (xpathReference: string): JRTreeReference => {
+	return new JRTreeReference(xpathReference);
+};

packages/scenario/src/jr/form/JRFormDef.ts

@@ -0,0 +1,13 @@
+import type { RootNode } from '@odk-web-forms/xforms-engine';
+
+/**
+ * @todo Hopefully we can keep this interface extremely minimal. It currently
+ * exists only to allow tests calling into it to type check.
+ */
+export class JRFormDef {
+	constructor(readonly instanceRoot: RootNode) {}
+
+	getMainInstance(): RootNode {
+		return this.instanceRoot;
+	}
+}

packages/scenario/src/jr/xpath/JREvaluationContext.ts

@@ -0,0 +1,14 @@
+/**
+ * This class is a stub, meant only to represent values of JavaRosa's
+ * `EvaluationContext` type (named here with a `JR` prefix to disambiguate it
+ * from a substantially different interface of the same name within
+ * `@odk-web-forms/xpath-engine` internals).
+ */
+export class JREvaluationContext {
+	/**
+	 * This property only serves as a means to have TypeScript treat the class and
+	 * instances of it as a nominal type (i.e. so that it won't be confused with
+	 * an empty object type).
+	 */
+	protected readonly IS_STUB_OF_JAVAROSA_EVALUATION_CONTEXT = true;
+}

packages/scenario/src/jr/xpath/JRTreeReference.ts

@@ -0,0 +1,7 @@
+/**
+ * @todo Hopefully we can keep this interface extremely minimal. It currently
+ * exists only to allow tests calling into it to type check.
+ */
+export class JRTreeReference {
+	constructor(readonly xpathReference: string) {}
+}

packages/scenario/src/jr/xpath/JRXPathNodeset.ts

@@ -0,0 +1,22 @@
+import type { JavaUtilList } from '../../java/util/List.ts';
+import type { JRTreeReference } from './JRTreeReference.ts';
+import type { XPathPathExprEval } from './XPathPathExprEval.ts';
+
+/**
+ * Not currently implemented. Types are provided to represent certain
+ * implementation details exercised in tests ported from JavaRosa, with the
+ * intent that those tests should pass a type check even where they'd otherwise
+ * be skipped (or otherwise expected to fail for reasons not related to
+ * implementation detail-specific assertions).
+ *
+ * This interface corresponds to JavaRosa's class named `XPathNodeset`, here
+ * prefixed with `JR` to preemptively disambiguate any usage of the name in web
+ * forms internals (for instance, within `@odk-web-forms/xpath`, where the name
+ * could reasonably be expected to find use).
+ *
+ * @see {@link XPathPathExprEval} for additional details.
+ */
+export interface JRXPathNodeset {
+	getReferences(): JavaUtilList<JRTreeReference>;
+	size(): number;
+}

packages/scenario/src/jr/xpath/XPathPathExpr.ts

@@ -0,0 +1,13 @@
+import { UnclearApplicabilityError } from '../../error/UnclearApplicabilityError.ts';
+import type { JREvaluationContext } from './JREvaluationContext.ts';
+import type { JRTreeReference } from './JRTreeReference.ts';
+
+/**
+ * @todo Hopefully we can keep this interface extremely minimal. It currently
+ * exists only to allow tests calling into it to type check.
+ */
+export class XPathPathExpr {
+	static getRefValue(_model: unknown, _ec: JREvaluationContext, _ref: JRTreeReference): string {
+		throw new UnclearApplicabilityError('exposure of XPath evaluation implementation details');
+	}
+}

packages/scenario/src/jr/xpath/XPathPathExprEval.ts

@@ -0,0 +1,27 @@
+import { UnclearApplicabilityError } from '../../error/UnclearApplicabilityError.ts';
+import type { JREvaluationContext } from './JREvaluationContext.ts';
+import type { JRTreeReference } from './JRTreeReference.ts';
+import type { JRXPathNodeset } from './JRXPathNodeset.ts';
+
+/**
+ * **PORTING NOTES**
+ *
+ * This is currently a stub of the class and methods of the same name/shape in
+ * —and as called within tests ported from—JavaRosa. The constructor will
+ * produce a valid instance, but calls to any of its methods will throw a
+ * runtime error.
+ *
+ * From a testing perspective, this class is understood to be used in assertions
+ * about certain aspects of implementation detail, corresponding to concepts we
+ * don't expose in the engine/client interface nor expect to expose in the
+ * foreseeable future.
+ *
+ * Where possible, ported tests which depend on instances of this class will be
+ * accompanied by a supplemental test addressing aspects of those tests which
+ * are clearly transferrable to more pertinent client-facing APIs.
+ */
+export class XPathPathExprEval {
+	eval(_treeReference: JRTreeReference, _evaluationContext: JREvaluationContext): JRXPathNodeset {
+		throw new UnclearApplicabilityError('exposure of XPath evaluation implementation details');
+	}
+}

packages/scenario/test/relevant.test.ts

@@ -11,8 +11,12 @@ import {
 	title,
 } from '@odk-web-forms/common/test/fixtures/xform-dsl/index.ts';
 import { describe, expect, it } from 'vitest';
+import { intAnswer } from '../src/answer/ExpectedIntAnswer.ts';
+import { stringAnswer } from '../src/answer/ExpectedStringAnswer.ts';
 import type { UntypedAnswer } from '../src/answer/UntypedAnswer.ts';
-import { Scenario } from '../src/jr/Scenario.ts';
+import { Scenario, getRef } from '../src/jr/Scenario.ts';
+import { XPathPathExpr } from '../src/jr/xpath/XPathPathExpr.ts';
+import { XPathPathExprEval } from '../src/jr/xpath/XPathPathExprEval.ts';
 
 describe('Relevance - TriggerableDagTest.java', () => {
 	/**
@@ -220,4 +224,187 @@ describe('Relevance - TriggerableDagTest.java', () => {
 			expect(scenario.getAnswerNode('/data/group/field')).toBeNonRelevant();
 		});
 	});
+
+	describe('relevance', () => {
+		/**
+		 * JR:
+		 *
+		 * Nodes can be nested differently in the model and body. The model structure is used
+		 * to determine relevance inheritance.
+		 */
+		it('is determined by model nesting', async () => {
+			const scenario = await Scenario.init(
+				'Some form',
+				html(
+					head(
+						title('Some form'),
+						model(
+							mainInstance(t('data id="some-form"', t('outernode'), t('group', t('innernode')))),
+							bind('/data/group').relevant('false()')
+						)
+					),
+					body(group('/data/group', input('/data/outernode'), input('/data/group/innernode')))
+				)
+			);
+
+			expect(scenario.getAnswerNode('/data/group')).toBeNonRelevant();
+			expect(scenario.getAnswerNode('/data/outernode')).toBeRelevant();
+			expect(scenario.getAnswerNode('/data/group/innernode')).toBeNonRelevant();
+		});
+	});
+
+	describe('non-relevant nodes', () => {
+		/**
+		 * **PORTING NOTES**
+		 *
+		 * 1. This is almost certainly testing implementation details. We do not
+		 *    expose anything like this in the engine/client interface, nor do we
+		 *    anticipate doing so. Insofar as there is behavior we'd want to test
+		 *    for correctness (and there is!), it's probably better expressed by
+		 *    observing the effect non-relevance has on the values produced. A brief
+		 *    attempt at an alternate expression of this test follows.
+		 *
+		 * 2. This exact form structure is not presently supported by the engine! It
+		 *    fails because there is a check for same-name/same-parent nodes which
+		 *    don't correspond to a repeat (range). It's worth discussing whether
+		 *    this is a form structure we expect to support, and what sorts of form
+		 *    design would produce a form with a similar shape.
+		 *
+		 * 3. Regardless of whether we intend to support forms of a similar shape,
+		 *    it's also important to observe the semantics of the `relevant`
+		 *    expression itself. The `position()` call is **rather likely** to be
+		 *    evaluated against the context of the `<bind nodeset>` expression.
+		 *    (It's also possible to interpret it as evaluated against the parent's
+		 *    children, i.e. `/data/*`, but this seems much less likely to be the
+		 *    case.) This wouldn't be surprising, it may even be exactly what's
+		 *    expected. But the current engine behavior evaluates bind expressions
+		 *    against the bound node itself, as the initial "context node" (in XPath
+		 *    semantic terms)... as such, a `position()` call will currently always
+		 *    return `1` (unless called against some other context established by
+		 *    preceding expression steps).
+		 */
+		it.fails('[is] are excluded from nodeset evaluation', async () => {
+			const scenario = await Scenario.init(
+				'Some form',
+				html(
+					head(
+						title('Some form'),
+						model(
+							mainInstance(
+								t(
+									'data id="some-form"',
+									// position() is one-based
+									t('node', t('value', '1')), // non-relevant
+									t('node', t('value', '2')), // non-relevant
+									t('node', t('value', '3')), // relevant
+									t('node', t('value', '4')), // relevant
+									t('node', t('value', '5')) // relevant
+								)
+							),
+							bind('/data/node').relevant('position() > 2'),
+							bind('/data/node/value').type('int')
+						)
+					),
+					body(group('/data/node', input('/data/node/value')))
+				)
+			);
+
+			// The XPathPathExprEval is used when evaluating the nodesets that the
+			// xpath functions declared in triggerable expressions need to operate
+			// upon. This assertion shows that non-relevant nodes are not included
+			// in the resulting nodesets
+			expect(
+				new XPathPathExprEval()
+					.eval(getRef('/data/node'), scenario.getEvaluationContext())
+					.getReferences()
+					.size()
+			).toBe(3);
+
+			// The method XPathPathExpr.getRefValue is what ultimately is used by
+			// triggerable expressions to extract the values they need to operate
+			// upon. The following assertion shows how extrating values from
+			// non-relevant nodes returns `null` values instead of the actual values
+			// they're holding
+			expect(
+				XPathPathExpr.getRefValue(
+					scenario.getFormDef().getMainInstance(),
+					scenario.getEvaluationContext(),
+					scenario.expandSingle(getRef('/data/node[2]/value'))
+				)
+			).toBe('');
+
+			// ... as opposed to the value that we can get by resolving the same
+			// reference with the main instance, which has the expected `2` value
+			expect(scenario.answerOf('/data/node[2]/value')).toEqualAnswer(intAnswer(2));
+		});
+
+		/**
+		 * **PORTING NOTES** (supplemental to test above)
+		 *
+		 * This test exercises different semantics than the test above ported from
+		 * JavaRosa, but would also serve to exercise the concept that non-relevance
+		 * excludes a node from evaluation: specifically by virtue of its value
+		 * being blank.
+		 *
+		 * Unfortunately, it also reveals a bug in the engine's relevance
+		 * computation logic! At a glance, it appears that:
+		 *
+		 * 1. The `calculate` is evaluated against the nodes' default values
+		 * 2. Before relevance is computed for those nodes
+		 * 3. Finally, failing to recompute the `calculate` once those nodes'
+		 *    non-relevance is established
+		 */
+		it.fails(
+			'is excluded from producing values in an evaluation (supplemental to previous test)',
+			async () => {
+				const scenario = await Scenario.init(
+					'Some form',
+					html(
+						head(
+							title('exclusion of non-relevant values'),
+							model(
+								mainInstance(
+									t(
+										'data id="exclusion-of-non-relevant-values"',
+										t('is-node-a-relevant', 'no'),
+										t('is-node-b-relevant', 'no'),
+										t('is-node-c-relevant', 'yes'),
+										t('is-node-d-relevant', 'yes'),
+										t('is-node-e-relevant', 'yes'),
+										// position() is one-based
+										t('node-a', t('value', '1')), // non-relevant
+										t('node-b', t('value', '2')), // non-relevant
+										t('node-c', t('value', '3')), // relevant
+										t('node-d', t('value', '4')), // relevant
+										t('node-e', t('value', '5')), // relevant,
+										t('node-x-concat') // calculates a concatenation of node-a through node-e
+									)
+								),
+								bind('/data/node-a').relevant("/data/is-node-a-relevant = 'yes'"),
+								bind('/data/node-b').relevant("/data/is-node-b-relevant = 'yes'"),
+								bind('/data/node-c').relevant("/data/is-node-c-relevant = 'yes'"),
+								bind('/data/node-d').relevant("/data/is-node-d-relevant = 'yes'"),
+								bind('/data/node-e').relevant("/data/is-node-e-relevant = 'yes'"),
+								bind('/data/node-x-concat').calculate(
+									'concat(/data/node-a, /data/node-b, /data/node-c, /data/node-d, /data/node-e)'
+								),
+								bind('/data/node/value').type('int')
+							)
+						),
+
+						body(
+							group('/data/node-a', input('/data/node-a/value')),
+							group('/data/node-b', input('/data/node-b/value')),
+							group('/data/node-c', input('/data/node-c/value')),
+							group('/data/node-d', input('/data/node-d/value')),
+							group('/data/node-e', input('/data/node-e/value')),
+							input('/data/node-x-concat')
+						)
+					)
+				);
+
+				expect(scenario.answerOf('/data/node-x-concat')).toEqualAnswer(stringAnswer('345'));
+			}
+		);
+	});
 });