Port SelectMultipleChoiceFilterTest.java

packages/scenario/src/answer/ExpectedDisplayTextAnswer.ts

@@ -0,0 +1,83 @@
+import type { JSONValue } from '@odk-web-forms/common/types/JSONValue.ts';
+import { ComparableAnswer } from './ComparableAnswer.ts';
+
+/**
+ * @todo (discuss/review below notes)
+ *
+ * **PORTING NOTES**
+ *
+ * ### 1. Naming, semantics_
+ *
+ * The intent of this class's name is to match the apparent intent in JavaRosa,
+ * where assertions are made against a question's "display text". In the cases
+ * ported so far, that includes apparent "display text" of `<select>`s where
+ * multiple values are comparable as a single string of comma-separated values.
+ *
+ * As such, it **currently** provides support for assertions where the expected
+ * value is described as a single string of comma-separated values, where those
+ * values correspond to the expected values of a `<select>`s itemset/"inline"
+ * items.
+ *
+ * Other cases may be handled as they're encountered while porting. Otherwise,
+ * we want to may consider a variety of changes in naming, expressed test
+ * semantics, or other means of making the ported usage less awkward.
+ *
+ * ### 2. Select item/choice ordering in assertions/comparisons
+ *
+ * After briefly looking at some initial failures of tests using this answer
+ * type, it seemed that they likely should have either passed, or at least
+ * passed the specific assertions they first failed on. Specifically, it seemed
+ * that the expected select items matched:
+ *
+ * - All of the expected values
+ * - No excess values
+ * - No assertion of label values which would potentially fail to match
+ *
+ * They only failed because the initial comparison was order-sensitive. In
+ * hindsight, unless there is some nuance in either the ODK XForms spec, or in
+ * expected Collect behavior, it seems to me that `<select>` values should
+ * always be set-like, and explicitly order-agnostic.
+ *
+ * This **may also** call into question the distinction between the current
+ * `toContainChoices`/`toContainChoicesInAnyOrder` custom assertions. It seems
+ * likely that order **does matter** when comparing _available choices for
+ * selection_ (or **could optionally matter**, where some tests may want to
+ * validate it and others may ignore it to focus on other details), but should
+ * not matter when comparing _selected choices to assert expected values in form
+ * state_.
+ *
+ * At any rate, this type currently sorts its input, which probably still isn't
+ * the right semantic approach. It may make more sense to have a specific
+ * {@link ComparableAnswer} interface for custom comparison logic, e.g. so that
+ * two distinct types of comparables representing multiple selected items can
+ * perform the most appropriate comparison for its own semantics. (Something
+ * almost exactly like this was explored in other iterations on the many and
+ * growing `Comparable*` types, but put on hold after a brief timebox when it
+ * felt like
+ * {@link https://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it | YAGNI}).
+ */
+export class ExpectedDisplayTextAnswer extends ComparableAnswer {
+	readonly values: readonly string[];
+	readonly stringValue: string;
+
+	constructor(selectValuesAsCSV: string) {
+		super();
+
+		/**
+		 * @see {@link ExpectedDisplayTextAnswer} notes on applicability of ordering
+		 * in assertions
+		 */
+		const values = selectValuesAsCSV.split(', ').sort();
+
+		this.values = values;
+		this.stringValue = values.join(' ');
+	}
+
+	override inspectValue(): JSONValue {
+		return this.values;
+	}
+}
+
+export const answerText = (selectValuesAsCSV: string): ExpectedDisplayTextAnswer => {
+	return new ExpectedDisplayTextAnswer(selectValuesAsCSV);
+};

packages/scenario/src/answer/SelectValuesAnswer.ts

@@ -0,0 +1,28 @@
+import type { JSONValue } from '@odk-web-forms/common/types/JSONValue.ts';
+import type { SelectNode } from '@odk-web-forms/xforms-engine';
+import { ComparableAnswer } from './ComparableAnswer.ts';
+
+/**
+ * Produces a value which may be **assigned** to a {@link SelectNode}, e.g.
+ * as part of a test's "act" phase.
+ */
+export class SelectValuesAnswer extends ComparableAnswer {
+	/**
+	 * @todo This is currently produced by joining the input string values with
+	 * whitespace. This is correct for `<select>`, and ideally corresponds to
+	 * calls relating to fields of that type. Is there a way we can safeguard that
+	 * it isn't called incorrectly for `<select1>` (where joining multiple values
+	 * is undesirable)?
+	 */
+	readonly stringValue: string;
+
+	constructor(readonly values: readonly string[]) {
+		super();
+
+		this.stringValue = values.join(' ');
+	}
+
+	override inspectValue(): JSONValue {
+		return this.values;
+	}
+}

packages/scenario/src/assertion/extensions/choices.ts

@@ -12,10 +12,11 @@ import {
 import type { JSONValue } from '@odk-web-forms/common/types/JSONValue.ts';
 import { expect } from 'vitest';
 import { ComparableChoice } from '../../choice/ComparableChoice.ts';
+import { ExpectedChoice } from '../../choice/ExpectedChoice.ts';
 import { SelectChoiceList } from '../../jr/select/SelectChoiceList.ts';
 
 const assertSelectList = instanceAssertion(SelectChoiceList);
-const assertChoicesArray = instanceArrayAssertion(ComparableChoice);
+const assertExpectedChoicesArray = instanceArrayAssertion(ExpectedChoice);
 
 type SelectChoices = Iterable<ComparableChoice>;
 
@@ -38,74 +39,111 @@ class InspectableChoices implements Inspectable {
 	}
 }
 
-const choiceExtensions = extendExpect(expect, {
-	toContainChoices: new AsymmetricTypedExpectExtension(
-		assertSelectList,
-		assertChoicesArray,
-		(actual, expected) => {
-			const actualChoices = mapChoices(actual, 'comparableValue');
-			const expectedChoices = mapChoices(expected, 'comparableValue');
-			const missing: string[] = [];
-			const outOfOrder: string[] = [];
+interface GetContainsChoicesResultOptions {
+	readonly inAnyOrder: boolean;
+}
 
-			let previousIndex = -1;
+const getContainsChoicesResult = (
+	actual: SelectChoiceList,
+	expected: readonly ExpectedChoice[],
+	options: GetContainsChoicesResultOptions
+): InspectableComparisonError | true => {
+	const actualChoices = Array.from(actual);
+	const actualIndexes = expected.map((expectedChoice) => {
+		return actualChoices.findIndex((actualChoice) => {
+			return expectedChoice.checkEquality(actualChoice).pass;
+		});
+	});
+	const missingChoices = actualIndexes.flatMap((actualIndex, expectedIndex) => {
+		if (actualIndex === -1) {
+			return expected[expectedIndex]!;
+		}
 
-			for (const value of expectedChoices) {
-				const valueIndex = actualChoices.indexOf(value);
+		return [];
+	});
 
-				if (valueIndex === -1) {
-					missing.push(value);
-					continue;
-				} else if (valueIndex < previousIndex) {
-					outOfOrder.push(value);
-				}
+	const { inAnyOrder } = options;
+	const baseErrorOptions = inAnyOrder ? { comparisonQualifier: 'in any order' } : {};
 
-				previousIndex = valueIndex;
-			}
+	let error: InspectableComparisonError | null = null;
+
+	if (missingChoices.length > 0) {
+		const missing = mapChoices(missingChoices, 'comparableValue');
 
-			const pass = missing.length === 0 && outOfOrder.length === 0;
-
-			return (
-				pass ||
-				new InspectableComparisonError(
-					new InspectableChoices(actual),
-					new InspectableChoices(expected),
-					'contain',
-					{
-						details: `Missing choices: ${JSON.stringify(missing)}\nChoices out of order: ${JSON.stringify(outOfOrder)}`,
-					}
-				)
+		error = new InspectableComparisonError(
+			new InspectableChoices(actual),
+			new InspectableChoices(expected),
+			'contain',
+			{
+				...baseErrorOptions,
+				details: `Missing choices: ${JSON.stringify(missing)}`,
+			}
+		);
+	} else if (!inAnyOrder) {
+		const isExpectedOrder = actualIndexes.every((actualIndex, i) => {
+			const previousIndex = actualIndexes[i - 1];
+
+			return previousIndex == null || actualIndex > previousIndex;
+		});
+
+		if (!isExpectedOrder) {
+			error = new InspectableComparisonError(
+				new InspectableChoices(actual),
+				new InspectableChoices(expected),
+				'contain',
+				{
+					...baseErrorOptions,
+					details: `Choices match in unexpected order: ${JSON.stringify(actualIndexes)}`,
+				}
 			);
 		}
+	}
+
+	return error == null || error;
+};
+
+const choiceExtensions = extendExpect(expect, {
+	toContainChoices: new AsymmetricTypedExpectExtension(
+		assertSelectList,
+		assertExpectedChoicesArray,
+		(actual, expected) => {
+			return getContainsChoicesResult(actual, expected, {
+				inAnyOrder: false,
+			});
+		}
 	),
+
+	/**
+	 * **NOTE FOR REVIEWERS**
+	 *
+	 * This could really go on any of the custom assertion extensions. But it's
+	 * going here, because here is where I experienced the wonder that prompted me
+	 * to write it.
+	 *
+	 * The general approach to custom assertion extensions felt a bit
+	 * overengineered as I was working on it. It simplifies and clarifies a few
+	 * things that felt confusing without a more purposeful abstraction, but it
+	 * could have been plenty good enough to just reference the docs, talk about
+	 * it, otherwise build a shared understanding of the Vitest extension API's
+	 * inherent complexities.
+	 *
+	 * I realized today that, entirely by lucky accident, this solution is so much
+	 * better than that. And I realized it because, again entirely by lucky
+	 * accident, I discovered that command-clicking (or otherwise using IDE
+	 * navigation) from an actual call to the assertion in any random test...
+	 * comes right back to this implementation. In hindsight, of course it does.
+	 * But that's a thing we'd give up without a little bit of extra abstraction
+	 * on top of Vitest, and it's a thing I think we'll find quite valuable if we
+	 * also find custom domain-specific assertions valuable for their
+	 * expressiveness in tests.
+	 */
 	toContainChoicesInAnyOrder: new AsymmetricTypedExpectExtension(
 		assertSelectList,
-		assertChoicesArray,
+		assertExpectedChoicesArray,
 		(actual, expected) => {
-			const actualValues = mapChoices(actual, 'comparableValue');
-			const expectedValues = mapChoices(expected, 'comparableValue');
-			const missingValues: string[] = [];
-
-			for (const value of expectedValues) {
-				if (!actualValues.includes(value)) {
-					missingValues.push(value);
-				}
-			}
-
-			const pass = missingValues.length === 0;
-
-			return (
-				pass ||
-				new InspectableComparisonError(
-					new InspectableChoices(actual),
-					new InspectableChoices(expected),
-					'contain',
-					{
-						comparisonQualifier: 'in any order',
-						details: `Missing choices: ${JSON.stringify(missingValues)}`,
-					}
-				)
-			);
+			return getContainsChoicesResult(actual, expected, {
+				inAnyOrder: true,
+			});
 		}
 	),
 });