Skip to content

Commit

Permalink
Update other tests now passing with boolean cast fix
Browse files Browse the repository at this point in the history
  • Loading branch information
@eyelidlessness
eyelidlessness committed Jul 2, 2024
1 parent cb1ea4f commit 8176705
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 140 deletions.
150 changes: 51 additions & 99 deletions packages/scenario/test/readonly.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,113 +14,65 @@ import { describe, expect, it } from 'vitest';
import { Scenario } from '../src/jr/Scenario.ts';

describe('TriggerableDagTest.java', () => {
interface CastReadonlyExpressionOptions {
readonly castReadonlyExpressionsAsNumber: boolean;
}

/**
* **PORTING NOTES**
*
* This test has the same semantic considerations (XPath node-set -> boolean
* casting, potential considerations for value casting on writes, etc) as the
* ported relevance test of a similar name/approach.
*
* The test has been parameterized to demonstrate this concisely, with the
* second run wrapping the `readonly` expressions in a `number()` cast (which
* passes as expected).
*
* JR:
*
* Read-only is inherited from ancestor nodes, as per the W3C XForms specs:
* - https://www.w3.org/TR/xforms11/#model-prop-relevant
*/
describe.each<CastReadonlyExpressionOptions>([
{ castReadonlyExpressionsAsNumber: false },
{ castReadonlyExpressionsAsNumber: true },
])(
'readonly (cast readonly expression as number: $castReadonlyExpressionAsNumber)',
({ castReadonlyExpressionsAsNumber }) => {
let testFn: typeof it | typeof it.fails;

if (castReadonlyExpressionsAsNumber) {
testFn = it;
} else {
testFn = it.fails;
}

let castReadonlyExpression: (baseExpression: string) => string;

if (castReadonlyExpressionsAsNumber) {
castReadonlyExpression = (baseExpression) => `number(${baseExpression})`;
} else {
castReadonlyExpression = (baseExpression) => baseExpression;
}

testFn('is inherited from ancestors', async () => {
const scenario = await Scenario.init(
'Some form',
html(
head(
title('Some form'),
model(
mainInstance(
t(
'data id="some-form"',
t('is-outer-readonly'),
t('is-inner-readonly'),
t('is-field-readonly'),
t('outer', t('inner', t('field')))
)
),
bind('/data/is-outer-readonly').type('boolean'),
bind('/data/is-inner-readonly').type('boolean'),
bind('/data/is-field-readonly').type('boolean'),
bind('/data/outer').readonly(castReadonlyExpression('/data/is-outer-readonly')),
bind('/data/outer/inner').readonly(
castReadonlyExpression('/data/is-inner-readonly')
),
bind('/data/outer/inner/field')
.type('string')
.readonly(castReadonlyExpression('/data/is-field-readonly'))
it('is inherited from ancestors', async () => {
const scenario = await Scenario.init(
'Some form',
html(
head(
title('Some form'),
model(
mainInstance(
t(
'data id="some-form"',
t('is-outer-readonly'),
t('is-inner-readonly'),
t('is-field-readonly'),
t('outer', t('inner', t('field')))
)
),
body(
input('/data/is-outer-readonly'),
input('/data/is-inner-readonly'),
input('/data/is-field-readonly'),
group('/data/outer', group('/data/outer/inner', input('/data/outer/inner/field')))
)
bind('/data/is-outer-readonly').type('boolean'),
bind('/data/is-inner-readonly').type('boolean'),
bind('/data/is-field-readonly').type('boolean'),
bind('/data/outer').readonly('/data/is-outer-readonly'),
bind('/data/outer/inner').readonly('/data/is-inner-readonly'),
bind('/data/outer/inner/field').type('string').readonly('/data/is-field-readonly')
)
);
),
body(
input('/data/is-outer-readonly'),
input('/data/is-inner-readonly'),
input('/data/is-field-readonly'),
group('/data/outer', group('/data/outer/inner', input('/data/outer/inner/field')))
)
)
);

// Form initialization evaluates all triggerables, which makes the field editable (not read-only)
expect(scenario.getInstanceNode('/data/outer')).toBeEnabled();
expect(scenario.getInstanceNode('/data/outer/inner')).toBeEnabled();
expect(scenario.getInstanceNode('/data/outer/inner/field')).toBeEnabled();
// Form initialization evaluates all triggerables, which makes the field editable (not read-only)
expect(scenario.getInstanceNode('/data/outer')).toBeEnabled();
expect(scenario.getInstanceNode('/data/outer/inner')).toBeEnabled();
expect(scenario.getInstanceNode('/data/outer/inner/field')).toBeEnabled();

// Make the outer group read-only
scenario.answer('/data/is-outer-readonly', true);
// Make the outer group read-only
scenario.answer('/data/is-outer-readonly', true);

expect(scenario.getInstanceNode('/data/outer')).toBeReadonly();
expect(scenario.getInstanceNode('/data/outer/inner')).toBeReadonly();
expect(scenario.getInstanceNode('/data/outer/inner/field')).toBeReadonly();
expect(scenario.getInstanceNode('/data/outer')).toBeReadonly();
expect(scenario.getInstanceNode('/data/outer/inner')).toBeReadonly();
expect(scenario.getInstanceNode('/data/outer/inner/field')).toBeReadonly();

// Make the inner group read-only
scenario.answer('/data/is-outer-readonly', false);
scenario.answer('/data/is-inner-readonly', true);
// Make the inner group read-only
scenario.answer('/data/is-outer-readonly', false);
scenario.answer('/data/is-inner-readonly', true);

expect(scenario.getInstanceNode('/data/outer')).toBeEnabled();
expect(scenario.getInstanceNode('/data/outer/inner')).toBeReadonly();
expect(scenario.getInstanceNode('/data/outer/inner/field')).toBeReadonly();
expect(scenario.getInstanceNode('/data/outer')).toBeEnabled();
expect(scenario.getInstanceNode('/data/outer/inner')).toBeReadonly();
expect(scenario.getInstanceNode('/data/outer/inner/field')).toBeReadonly();

// Make the field read-only
scenario.answer('/data/is-inner-readonly', false);
scenario.answer('/data/is-field-readonly', true);
// Make the field read-only
scenario.answer('/data/is-inner-readonly', false);
scenario.answer('/data/is-field-readonly', true);

expect(scenario.getInstanceNode('/data/outer')).toBeEnabled();
expect(scenario.getInstanceNode('/data/outer/inner')).toBeEnabled();
expect(scenario.getInstanceNode('/data/outer/inner/field')).toBeReadonly();
});
}
);
expect(scenario.getInstanceNode('/data/outer')).toBeEnabled();
expect(scenario.getInstanceNode('/data/outer/inner')).toBeEnabled();
expect(scenario.getInstanceNode('/data/outer/inner/field')).toBeReadonly();
});
});
52 changes: 11 additions & 41 deletions packages/scenario/test/relevant.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import {
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, getRef } from '../src/jr/Scenario.ts';
import { XPathPathExpr } from '../src/jr/xpath/XPathPathExpr.ts';
import { XPathPathExprEval } from '../src/jr/xpath/XPathPathExprEval.ts';
Expand All @@ -29,26 +28,7 @@ describe('Relevance - TriggerableDagTest.java', () => {
*/

describe('non-relevance', () => {
/**
* **PORTING NOTES**
*
* - This fails because the `relevant` expressions produce node-sets, which
* will always evaluate to `true` when those nodes are present (which they
* always are in this test).
*
* - Those node-sets evaluate to nodes which are bound with `<bind
* type="boolean" />`, which strongly suggests that a bind's data type
* should influence casting semantics in expressions like `relevant`, and
* perhaps more generally.
*
* - There are some unaddressed casting considerations which **might be**
* implied by this, discussed in greater detail in porting notes on
* {@link UntypedAnswer}.
*
* Two additional variants of this test are added immediately following this
* one, both briefly exploring some of the contours of the current failure.
*/
it.fails('is inherited from ancestors', async () => {
it('is inherited from ancestors', async () => {
const scenario = await Scenario.init(
'Some form',
html(
Expand Down Expand Up @@ -102,23 +82,13 @@ describe('Relevance - TriggerableDagTest.java', () => {
* **PORTING NOTES** (first variant of ported test above)
*
* This test is identical to the test above, except that both `relevant`
* expressions are wrapped in a `string()` XPath call. The test still fails,
* but notably the failing assertion comes later:
*
* In the original test, the first assertion fails because a `node-set`
* expression which resolves to any node will always cast to `true`. When
* the value is cast to a string, the node's text value is consulted in
* casting, producing `false` when empty.
*
* Ultimately, the test fails when checking restoration of the `false`
* state. This is because the `false` value is presently being persisted to
* the primary instance as the string `"0"` (which, as I recall, is the
* expected serialization of boolean `false`). Since the `relevant`
* expression itself produces a string value, and with the engine still
* following strict XPath casting semantics, the value `"0"` is also cast to
* boolean `true` (again, consistent with XPath semantics).
* expressions are wrapped in a `string()` XPath call. This test was
* originally introduced to demonstrate different behavior of boolean
* casting logic depending on the expression's result type. This distinction
* no longer exists, but the alternate test is preserved to help us catch
* potential regressions in boolean casting logic.
*/
it.fails('is inherited from ancestors (variant #1: node-set semantics -> string)', async () => {
it('is inherited from ancestors (variant #1: node-set semantics -> string)', async () => {
const scenario = await Scenario.init(
'Some form',
html(
Expand Down Expand Up @@ -172,10 +142,10 @@ describe('Relevance - TriggerableDagTest.java', () => {
* **PORTING NOTES** (second variant)
*
* This variant of the ported JavaRosa test again casts the `relevant`
* expressions, this time to `number`. Here we see the test passes! This
* variant is included because it demonstrates all of the findings above, by
* showing how strict XPath casting semantics interact with the test form's
* expected XForms semantics.
* expressions, this time to `number`. Like the previous variant, this test
* was originally introduced to demonstrate differences in casting behavior
* depending on the expression's result type. It is likewise preserved to
* help us avoid casting-related regressions in the future.
*/
it('is inherited from ancestors (variant #2: node-set semantics -> number)', async () => {
const scenario = await Scenario.init(
Expand Down

0 comments on commit 8176705

Please sign in to comment.