choice randomization: better approximation of JR behaviour, fixes #49

packages/xpath/src/lib/collections/sort.ts

@@ -15,6 +15,7 @@ class UnseededPseudoRandomNumberGenerator implements PseudoRandomNumberGenerator
 }
 
 class SeededPseudoRandomNumberGenerator implements PseudoRandomNumberGenerator {
+	// Park-Miller PRNG
 	protected seed: number;
 
 	constructor(seed: Int) {
@@ -38,17 +39,46 @@ class SeededPseudoRandomNumberGenerator implements PseudoRandomNumberGenerator {
 	}
 }
 
-const isInt = (value: number): value is Int => value % 1 === 0;
-
 export const seededRandomize = <T>(values: readonly T[], seed?: number): T[] => {
 	let generator: PseudoRandomNumberGenerator;
 
 	if (seed == null) {
 		generator = new UnseededPseudoRandomNumberGenerator();
-	} else if (!isInt(seed)) {
-		throw 'todo not an int';
 	} else {
-		generator = new SeededPseudoRandomNumberGenerator(seed);
+		let finalSeed: number;
+		// Per issue #49 this is (to an extent) "bug-or-feature-compatible" with JavaRosa's implementation.
+		// org.javarosa.core.model.ItemsetBinding.resolveRandomSeed takes the .longValue() of
+		// the double produced by randomSeedPathExpr.eval().
+		// That results in a 0L when the double is NaN, which happens (for instance) when there
+		// is a string that does not look like a number (which is a problem in itself, as any non-numeric
+		// looking string will then result in the same seed of 0 — see https://github.com/getodk/javarosa/issues/800).
+		// We'll emulate Java's Double -> Long conversion here (for NaN and some other double values)
+		// so that we produce the same randomization as JR.
+		if (Number.isNaN(seed)) {
+			finalSeed = 0;
+		} else if (seed === Infinity) {
+			// In Java's .longValue(), this converts to 2**63 -1.
+			// But that's larger than the JS Number.MAX_SAFE_INTEGER, and thus we cannot guarantee the same
+			// outcomes as OpenRosa.
+			// However. When Park-Miller is initialized, it takes the modulus of the seed and 2**31 -1 as
+			// the first step. This means that for Park-Miller we can use 2**31 (which is smaller than Number.MAX_SAFE_INTEGER)
+			// as a surrogate equivalent seed for Infinity, since
+			// ((2**63 -1) % (2**31 -1)) = ((2**31) % (2**31 -1))
+			// (because of JS Number imprecision (the problem to start with) don't use JS to convince of the above equality,
+			// or rewrite to use BigInt).
+			finalSeed = 2 ** 31;
+		} else if (seed === -Infinity) {
+			// Analogous with the above conversion for Infinity
+			finalSeed = -(2 ** 31 + 1);
+		} else if (!Number.isInteger(seed)) {
+			// We're not out of the woods yet — see issue: https://github.com/getodk/web-forms/issues/240.
+			// But one thing we know is that JR converts the double to a long, and thus drops the fractional part.
+			// We'll do the same here.
+			finalSeed = Math.floor(seed);
+		} else {
+			finalSeed = seed;
+		}
+		generator = new SeededPseudoRandomNumberGenerator(finalSeed);
 	}
 
 	const { length } = values;

packages/xpath/test/xforms/randomize.test.ts

@@ -74,6 +74,11 @@ describe('randomize()', () => {
 			{ seed: 1, expected: 'BFEACD' },
 			{ seed: 11111111, expected: 'ACDBFE' },
 			{ seed: 'int(1)', expected: 'BFEACD' },
+			{ seed: 1.1, expected: 'BFEACD' },
+			{ seed: 0, expected: 'CBEAFD' },
+			{ seed: NaN, expected: 'CBEAFD' },
+			{ seed: Infinity, expected: 'CBEAFD' },
+			{ seed: -Infinity, expected: 'CBEAFD' },
 			{ seed: 'floor(1.1)', expected: 'BFEACD' },
 			{ seed: '//xhtml:div[@id="testFunctionNodeset2"]/xhtml:p', expected: 'BFEACD' },
 		].forEach(({ seed, expected }) => {
@@ -90,10 +95,9 @@ describe('randomize()', () => {
 
 	[
 		{ expression: 'randomize()' },
-		{ expression: `randomize(${SELECTOR}, 'a')` },
 		{ expression: `randomize(${SELECTOR}, 1, 2)` },
 	].forEach(({ expression }) => {
-		it.fails(`${expression} with invalid args, throws an error`, () => {
+		it.fails(`${expression} with invalid argument count, throws an error`, () => {
 			testContext.evaluate(expression);
 		});
 	});