Skip to content

Commit

Permalink
update tests, remove extraneous assertion logic
Browse files Browse the repository at this point in the history
  • Loading branch information
Chris Garrett committed Jan 24, 2020
1 parent 295312b commit 1b06a4f
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 123 deletions.
61 changes: 9 additions & 52 deletions packages/@glimmer/validator/lib/validators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,7 @@ export interface Tagged {
*
* @param tag
*/
export function value(tag: Tag): Revision {
if (DEBUG) {
// compute to cache the latest value, which will prevent us from doing
// invalid updates later on.
tag[COMPUTE]();
}

export function value(_tag: Tag): Revision {
return $REVISION;
}

Expand All @@ -71,27 +65,9 @@ export function value(tag: Tag): Revision {
* @param snapshot
*/
export function validate(tag: Tag, snapshot: Revision) {
if (DEBUG) {
IS_VALIDATING = true;
}

let isValid = snapshot >= tag[COMPUTE]();

if (DEBUG) {
IS_VALIDATING = false;

if (isValid) {
// compute to cache the latest value, which will prevent us from doing
// invalid updates later on.
tag[COMPUTE]();
}
}

return isValid;
return snapshot >= tag[COMPUTE]();
}

let IS_VALIDATING: boolean | undefined;

//////////

/**
Expand Down Expand Up @@ -143,7 +119,7 @@ class MonomorphicTagImpl implements MonomorphicTag {
private subtags: Tag[] | null = null;

private subtag: Tag | null = null;
private subtagCachedValue: Revision | null = null;
private subtagBufferCache: Revision | null = null;

[TYPE]: MonomorphicTagType;

Expand All @@ -162,38 +138,19 @@ class MonomorphicTagImpl implements MonomorphicTag {
this.lastChecked = ++$REVISION;
} else if (lastChecked !== $REVISION) {
this.isUpdating = true;

if (DEBUG) {
// In DEBUG, we don't cache while validating only, because it is valid
// update a tag between calling `validate()` and `value()`. Once you
// call `value()` on a tag, its revision is effectively locked in, and
// if you attempt to update it to a tag that is more recent it could
// break assumptions in our system. This is why the assertion exists in
// the static `update()` method below.
if (!IS_VALIDATING) {
this.lastChecked = $REVISION;
}
} else {
this.lastChecked = $REVISION;
}
this.lastChecked = $REVISION;

try {
let {
subtags,
subtag,
subtagCachedValue,
lastValue,
revision
} = this;
let { subtags, subtag, subtagBufferCache, lastValue, revision } = this;

if (subtag !== null) {
let subtagValue = subtag[COMPUTE]();

if (subtagValue === subtagCachedValue) {
if (subtagValue === subtagBufferCache) {
revision = Math.max(revision, lastValue);
} else {
// Clear the temporary cache
this.subtagCachedValue = null;
// Clear the temporary buffer cache
this.subtagBufferCache = null;
revision = Math.max(revision, subtagValue);
}
}
Expand Down Expand Up @@ -244,7 +201,7 @@ class MonomorphicTagImpl implements MonomorphicTag {
// subsequent updates. If its value hasn't changed, then we return the
// parent's previous value. Once the subtag changes for the first time,
// we clear the cache and everything is finally in sync with the parent.
tag.subtagCachedValue = subtag[COMPUTE]();
tag.subtagBufferCache = subtag[COMPUTE]();
tag.subtag = subtag;
}
}
Expand Down
100 changes: 29 additions & 71 deletions packages/@glimmer/validator/test/validators-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,91 +74,49 @@ module('@glimmer/validator: validators', () => {
assert.ok(validate(tag, snapshot));
});

if (DEBUG) {
test('it throws an error when updating a tag that has been validated with a tag that is more recent', assert => {
let tag = createUpdatableTag();
let subtag = createUpdatableTag();

// First, we get a snapshot of the parent
let snapshot = value(tag);

// Then we dirty the currently unrelated subtag
dirty(subtag);

// Then we validate the parent tag. This should "lock in" the tag, because it
// means we are verifying that whatever calculation it represents has not changed
assert.ok(validate(tag, snapshot));

// Now, we attempt to update the parent with the subtag. The parent has been locked
// in, but the subtag has a more recent value, which means we have backflowed. This
// should throw an error.
assert.throws(
() => update(tag as any, subtag),
/Error: BUG: attempted to update a tag with a tag that has a more recent revision as its value/
);
});

test('it throws an error when updating a tag that has been invalidated AND whose value has been read with a tag that is more recent', assert => {
let tag = createUpdatableTag();
let subtag = createUpdatableTag();

// First, we get a snapshot of the parent
let snapshot = value(tag);

// Then we dirty the parent so it is out of date
dirty(tag);
test('it correctly buffers updates when subtag has a less recent value', assert => {
let tag = createUpdatableTag();
let subtag = createUpdatableTag();

// Then we dirty the currently unrelated subtag, so it has a more recent value
dirty(subtag);
// First, we dirty the parent tag so it is more recent than the subtag
dirty(tag);

// Then we attempt validate the parent tag. The validation will fail, and will
// NOT lock in the parent value just yet.
assert.notOk(validate(tag, snapshot));
// Then, we get a snapshot of the parent
let snapshot = value(tag);

// Then we get the parent tag's value, which does lock in the value.
snapshot = value(tag);
// Now, we update the parent tag with the subtag, and revalidate it
update(tag as any, subtag);

// Now, we attempt to update the parent with the subtag. The parent has been locked
// in, but the subtag has a more recent value, which means we have backflowed. This
// should throw an error.
assert.throws(
() => update(tag as any, subtag),
/Error: BUG: attempted to update a tag with a tag that has a more recent revision as its value/
);
});
assert.ok(validate(tag, snapshot), 'tag is still valid after being updated');

test('does not throw an error when updating a tag that has been invalidated AND whose value has NOT been read with a tag that is more recent', assert => {
let tag = createUpdatableTag();
let subtag = createUpdatableTag();
// Finally, dirty the subtag one final time to bust the buffer cache
dirty(subtag);

// First, we get a snapshot of the parent
let snapshot = value(tag);
assert.notOk(validate(tag, snapshot), 'tag is invalid after subtag is dirtied again');
});

// Then we dirty the parent so it is out of date
dirty(tag);
test('it correctly buffers updates when subtag has a more recent value', assert => {
let tag = createUpdatableTag();
let subtag = createUpdatableTag();

// Then we dirty the currently unrelated subtag, so it has a more recent value
dirty(subtag);
// First, we get a snapshot of the parent
let snapshot = value(tag);

// Then we attempt validate the parent tag. The validation will fail, and will
// NOT lock in the parent value just yet.
assert.notOk(validate(tag, snapshot));
// Then we dirty the currently unrelated subtag
dirty(subtag);

// Now we update the parent to the new subtag, BEFORE its value has been
// calculated. Since we know the value hasn't been calculated or locked into
// the cache, this update is valid.
update(tag as any, subtag);
// Now, we update the parent tag with the subtag, and revalidate it
update(tag as any, subtag);

// Then we get the parent tag's value, which does lock in the value.
snapshot = value(tag);
assert.ok(validate(tag, snapshot), 'tag is still valid after being updated');

// Do an unrelated bump (something else in the system changed)
bump();
// Finally, dirty the subtag one final time to bust the buffer cache
dirty(subtag);

// Caches were fully busted, but the value was still correct.
assert.ok(validate(tag, snapshot));
});
assert.notOk(validate(tag, snapshot), 'tag is invalid after subtag is dirtied again');
});

if (DEBUG) {
test('does not allow cycles on tags that have not been marked with ALLOW_CYCLES', assert => {
let tag = createUpdatableTag();
let subtag = createUpdatableTag();
Expand Down

0 comments on commit 1b06a4f

Please sign in to comment.