Skip to content

Commit

Permalink
recipe2plan 2/n: Validate Handles, Tests (#4834)
Browse files Browse the repository at this point in the history 
* creating structure of ts r2p script

* Created fast CLI rapper for r2p script

* WIP figuring out ways to resolve a recipe

* fix sp

* Passing lint, still WIP

* Build macros for recipe2plan work

* Lightweight iteration on recipe2plan, stubbed out tests

* Revised method to find corresponding create handles

* Added type info

* Added method to get all handles to manifest + get all handles by Id

* fix: no flatMap

* - currently, fails since it cannot find associated stores for handles.

* uncleaned, but working recipe resolution

* Cleaned up for r2p, pt 1

* stubbed out second test (short, short)

* Squashed commit of the following:

commit 62dcc57
Author: Alex Rosengarten <alxrsngrtn@google.com>
Date:   Mon Mar 2 14:38:14 2020 -0800

    Fixed test, added TODO

* updates to test, can't get two to fail

* Squashed commit of the following:

commit bede8d9
Author: Alex Rosengarten <alxrsngrtn@google.com>
Date:   Mon Mar 2 14:52:02 2020 -0800

    Fixed build rule, simplified runtime

commit 62dcc57
Author: Alex Rosengarten <alxrsngrtn@google.com>
Date:   Mon Mar 2 14:38:14 2020 -0800

    Fixed test, added TODO

* can't get test to fail... hmm...

* Squashed commit of the following:

commit f6df54a
Author: Alex Rosengarten <alxrsngrtn@google.com>
Date:   Mon Mar 2 15:44:32 2020 -0800

    tools/sigh lint

commit bede8d9
Author: Alex Rosengarten <alxrsngrtn@google.com>
Date:   Mon Mar 2 14:52:02 2020 -0800

    Fixed build rule, simplified runtime

commit 62dcc57
Author: Alex Rosengarten <alxrsngrtn@google.com>
Date:   Mon Mar 2 14:38:14 2020 -0800

    Fixed test, added TODO

* adjusting recipe copying to include triggers

* Revert "adjusting recipe copying to include triggers"

This reverts commit 9bc3734

* four tests complete

* Added test, fixed minor async issue

* Squashed commit of the following:

commit 4b05a28
Merge: f4c1d26 99000c2
Author: Alex Rosengarten <alxrsngrtn@google.com>
Date:   Tue Mar 3 11:18:14 2020 -0800

    Merge branch 'r2p' of github.com:alxrsngrtn/arcs into r2p

commit f4c1d26
Merge: 140111f 5c6310d
Author: Alex Rosengarten <alxrsngrtn@google.com>
Date:   Tue Mar 3 11:14:35 2020 -0800

    Merge branch 'master' of github.com:PolymerLabs/arcs into r2p
    Fixed lint errors

commit 99000c2
Merge: 140111f 5c6310d
Author: Alex Rosengarten <alxrsngrtn@google.com>
Date:   Tue Mar 3 11:14:35 2020 -0800

    Merge branch 'master' of github.com:PolymerLabs/arcs into r2p
    Fixed lint errors

commit 140111f
Author: Alex Rosengarten <alxrsngrtn@google.com>
Date:   Tue Mar 3 11:08:12 2020 -0800

    Removed flatMap, added TODOs and link to GH issue

commit 5c6310d
Author: jblebrun <jibbl@google.com>
Date:   Tue Mar 3 11:02:34 2020 -0800

    Simplify service test pattern (#4817)

    * Make a simple test lifecycle registry, instead of creating empty
    testactivity

    * Remove use of `runBlockingTest`: according to
    Kotlin/kotlinx.coroutines#1222, if the test
    results in coroutines being finished on other threads, it's possible to
    receive "This job has not yet completed" exceptions, even though the
    jobs were properly terminated. Since we don't need the delay-skipping
    properties of `runBlockingTest`, I think it's OK to use `runBlocking`.

commit 472bc84
Author: Alex Rosengarten <alxrsngrtn@google.com>
Date:   Tue Mar 3 10:27:56 2020 -0800

    Improved build rules

commit ca1ebf8
Author: Alex Rosengarten <alxrsngrtn@google.com>
Date:   Tue Mar 3 10:17:13 2020 -0800

    Impl suggestsions for r2p

commit b5578ea
Author: Jason Feinstein <jwf@google.com>
Date:   Tue Mar 3 09:54:17 2020 -0800

    Add tests for dereferencing references to the HandleManagerTest(s) (#4816)

    * Add dereferencing tests to the android handle manager test.

    * Add dereferencing tests to core HandleManager.

commit a695797
Author: Jason Feinstein <jwf@google.com>
Date:   Mon Mar 2 18:14:07 2020 -0800

    Create RawEntityDereferencerTest, storage Reference-> CrdtEntity.Reference (#4812)

    * Create RawEntityDereferencerTest, make storage Reference implement CrdtEntity.Reference.

    Also: Create ParcelableReference.

    * Add dep. Also, apparently read/writeBoolean is Q-only.

    * Just write null if there is no version map.

    * Just write null if there is no version map.

commit ba7a107
Author: Gogul Balakrishnan <bgogul@google.com>
Date:   Mon Mar 2 17:51:35 2020 -0800

    Add a decoder for PrimitiveTypeProto and an option to disable android constraints in BUILD. (#4793)

commit f6df54a
Author: Alex Rosengarten <alxrsngrtn@google.com>
Date:   Mon Mar 2 15:44:32 2020 -0800

    tools/sigh lint

commit bede8d9
Author: Alex Rosengarten <alxrsngrtn@google.com>
Date:   Mon Mar 2 14:52:02 2020 -0800

    Fixed build rule, simplified runtime

commit 62dcc57
Author: Alex Rosengarten <alxrsngrtn@google.com>
Date:   Mon Mar 2 14:38:14 2020 -0800

    Fixed test, added TODO

* fix ineq

* existing tests passing

* Updating invalid type test

* Squashed commit of the following:

commit b3d92d0
Author: Alex Rosengarten <alxrsngrtn@google.com>
Date:   Wed Mar 4 12:17:17 2020 -0800

    updated test name

commit ba7884b
Author: Alex Rosengarten <alxrsngrtn@google.com>
Date:   Wed Mar 4 12:16:09 2020 -0800

    nested unit tests

commit ef36fdf
Author: Alex Rosengarten <alxrsngrtn@google.com>
Date:   Wed Mar 4 12:15:00 2020 -0800

    renamed to tryResolve

commit 5703f02
Author: Alex Rosengarten <alxrsngrtn@google.com>
Date:   Wed Mar 4 12:07:18 2020 -0800

    rm generator

commit 658cb1a
Author: Alex Rosengarten <alxrsngrtn@google.com>
Date:   Wed Mar 4 12:01:54 2020 -0800

    implemented more review suggestions

commit 0056ee3
Merge: cae89c3 0f0f82a
Author: Alex Rosengarten <alxrsngrtn@google.com>
Date:   Wed Mar 4 11:54:23 2020 -0800

    Merge branch 'master' of github.com:PolymerLabs/arcs into r2p

commit 0f0f82a
Author: Maria Mandlis <mmandlis@chromium.org>
Date:   Wed Mar 4 09:33:29 2020 -0800

    add queryable capability (#4794)

commit d032c75
Author: Maria Mandlis <mmandlis@chromium.org>
Date:   Wed Mar 4 09:23:07 2020 -0800

    add creatimeTimestamp to entities (kt) (#4823)

commit 30267fc
Author: jblebrun <jibbl@google.com>
Date:   Wed Mar 4 09:09:39 2020 -0800

    Remove some unused deps (#4831)

commit c519181
Author: Cameron Silvestrini <csilvestrini@users.noreply.github.com>
Date:   Wed Mar 4 17:14:07 2020 +1100

    Increase size of DatabaseImplTest to medium (#4827)

    * Increase size of DatabaseImplTest to medium

    Was a bit flaky.

    * Check flakiness

    * Revert presubmit tweak

commit fa621c6
Author: jblebrun <jibbl@google.com>
Date:   Tue Mar 3 22:05:07 2020 -0800

    Fix a race condition when setting up `ServiceStore` message channel (#4826)

    This seems to be the cause of the flakiness in `AndroidAllocatorTest`
    and `AndroidHandleManagerTest` (#4781)

commit 78bf4f0
Author: Gogul Balakrishnan <bgogul@google.com>
Date:   Tue Mar 3 18:00:49 2020 -0800

    Utility to convert type proto to field type (if possible). (#4814)

commit cae89c3
Merge: 0360752 e587539
Author: Alex Rosengarten <alxrsngrtn@google.com>
Date:   Tue Mar 3 15:50:34 2020 -0800

    Merge branch 'master' of github.com:PolymerLabs/arcs into r2p

commit e587539
Author: jblebrun <jibbl@google.com>
Date:   Tue Mar 3 15:07:16 2020 -0800

    Apply simplified test pattern tests in `arcs.android.host` (#4821)

    As in 5c6310d

commit 1af1aac
Author: Joshua Pratt <jp10010101010000@gmail.com>
Date:   Wed Mar 4 09:48:15 2020 +1100

    Multinomials (#4804)

    * multinomial

    * added tests

    * making constant readable again

    * reindent

    * added multivariate rearrangement tests

    * updated comment

    * pr comments:

    Co-authored-by: Ragav Sachdeva <ragavsachdeva007@gmail.com>

commit 0360752
Author: Alex Rosengarten <alxrsngrtn@google.com>
Date:   Tue Mar 3 11:42:31 2020 -0800

    fix, bad comparison op

commit 4b05a28
Merge: f4c1d26 99000c2
Author: Alex Rosengarten <alxrsngrtn@google.com>
Date:   Tue Mar 3 11:18:14 2020 -0800

    Merge branch 'r2p' of github.com:alxrsngrtn/arcs into r2p

commit f4c1d26
Merge: 140111f 5c6310d
Author: Alex Rosengarten <alxrsngrtn@google.com>
Date:   Tue Mar 3 11:14:35 2020 -0800

    Merge branch 'master' of github.com:PolymerLabs/arcs into r2p
    Fixed lint errors

commit 99000c2
Merge: 140111f 5c6310d
Author: Alex Rosengarten <alxrsngrtn@google.com>
Date:   Tue Mar 3 11:14:35 2020 -0800

    Merge branch 'master' of github.com:PolymerLabs/arcs into r2p
    Fixed lint errors

commit 140111f
Author: Alex Rosengarten <alxrsngrtn@google.com>
Date:   Tue Mar 3 11:08:12 2020 -0800

    Removed flatMap, added TODOs and link to GH issue

commit 5c6310d
Author: jblebrun <jibbl@google.com>
Date:   Tue Mar 3 11:02:34 2020 -0800

    Simplify service test pattern (#4817)

    * Make a simple test lifecycle registry, instead of creating empty
    testactivity

    * Remove use of `runBlockingTest`: according to
    Kotlin/kotlinx.coroutines#1222, if the test
    results in coroutines being finished on other threads, it's possible to
    receive "This job has not yet completed" exceptions, even though the
    jobs were properly terminated. Since we don't need the delay-skipping
    properties of `runBlockingTest`, I think it's OK to use `runBlocking`.

commit 472bc84
Author: Alex Rosengarten <alxrsngrtn@google.com>
Date:   Tue Mar 3 10:27:56 2020 -0800

    Improved build rules

commit ca1ebf8
Author: Alex Rosengarten <alxrsngrtn@google.com>
Date:   Tue Mar 3 10:17:13 2020 -0800

    Impl suggestsions for r2p

commit b5578ea
Author: Jason Feinstein <jwf@google.com>
Date:   Tue Mar 3 09:54:17 2020 -0800

    Add tests for dereferencing references to the HandleManagerTest(s) (#4816)

    * Add dereferencing tests to the android handle manager test.

    * Add dereferencing tests to core HandleManager.

commit a695797
Author: Jason Feinstein <jwf@google.com>
Date:   Mon Mar 2 18:14:07 2020 -0800

    Create RawEntityDereferencerTest, storage Reference-> CrdtEntity.Reference (#4812)

    * Create RawEntityDereferencerTest, make storage Reference implement CrdtEntity.Reference.

    Also: Create ParcelableReference.

    * Add dep. Also, apparently read/writeBoolean is Q-only.

    * Just write null if there is no version map.

    * Just write null if there is no version map.

commit ba7a107
Author: Gogul Balakrishnan <bgogul@google.com>
Date:   Mon Mar 2 17:51:35 2020 -0800

    Add a decoder for PrimitiveTypeProto and an option to disable android constraints in BUILD. (#4793)

commit f6df54a
Author: Alex Rosengarten <alxrsngrtn@google.com>
Date:   Mon Mar 2 15:44:32 2020 -0800

    tools/sigh lint

commit bede8d9
Author: Alex Rosengarten <alxrsngrtn@google.com>
Date:   Mon Mar 2 14:52:02 2020 -0800

    Fixed build rule, simplified runtime

commit 62dcc57
Author: Alex Rosengarten <alxrsngrtn@google.com>
Date:   Mon Mar 2 14:38:14 2020 -0800

    Fixed test, added TODO

* fixed nested tests

* rewording, reorganizing

* assert more ergonomic
  • Loading branch information
@alxmrs
alxmrs committed Mar 4, 2020
1 parent 298fdcc commit 337b7b2
Show file tree
Hide file tree
Showing 2 changed files with 123 additions and 21 deletions.
18 changes: 9 additions & 9 deletions src/tools/storage-key-recipe-resolver.ts
View file
Expand Up @@ -32,13 +32,14 @@ export class StorageKeyRecipeResolver {
/**
* Produces resolved recipes with storage keys.
*
* TODO(alxr): Apply to long-running recipes appropriately.
* TODO(#4818) Add passes to assign storage keys.
* @throws Error if recipe fails to resolve on first or second pass.
* @yields Resolved recipes with storage keys
*/
async resolve(): Promise<Recipe[]> {
const recipes = [];
for (const recipe of this.runtime.context.allRecipes) {
this.validateHandles(recipe);
const arc = this.runtime.newArc(this.getArcId(recipe), ramDiskStorageKeyPrefixForTest());
const opts = {errors: new Map<Recipe | RecipeComponent, string>()};
const resolved = await this.tryResolve(recipe, arc, opts);
Expand All @@ -63,7 +64,8 @@ export class StorageKeyRecipeResolver {
*/
async tryResolve(recipe: Recipe, arc: Arc, opts?: IsValidOptions): Promise<Recipe | null> {
const normalized = recipe.clone();
normalized.normalize();
const successful = normalized.normalize(opts);
if (!successful) return null;
if (normalized.isResolved()) return normalized;

return await (new RecipeResolver(arc).resolve(recipe, opts));
Expand Down Expand Up @@ -97,14 +99,14 @@ export class StorageKeyRecipeResolver {
}

/**
* TODO(#4818) method to match `map` and `copy` fated handles with storage keys from `create` handles.
* Checks that handles are existent, disambiguous, and initiated by a long-running arc.
*
* @throws when a mapped handle is associated with too many stores (ambiguous mapping).
* @throws when a mapped handle isn't associated with any store (no matches found).
* @throws when handle is mapped to a handle from an ephemeral recipe.
* @throws when a map or copy handle is associated with too many stores (ambiguous mapping).
* @throws when a map or copy handle isn't associated with any store (no matches found).
* @throws when a map or copy handle is associated with a handle from an ephemeral recipe.
* @param recipe long-running or ephemeral recipe
*/
matchKeysToHandles(recipe: Recipe) {
validateHandles(recipe: Recipe) {
recipe.handles
.filter(h => h.fate === 'map' || h.fate === 'copy')
.forEach(handle => {
Expand All @@ -121,8 +123,6 @@ export class StorageKeyRecipeResolver {
if (!match.recipe.isLongRunning) {
throw Error(`Handle ${handle.localName} mapped to ephemeral handle ${match.localName}.`);
}

handle.storageKey = match.storageKey;
});
}
}
126 changes: 114 additions & 12 deletions src/tools/tests/storage-key-recipe-resolver-test.ts
View file
Expand Up @@ -11,10 +11,11 @@
import {Manifest} from '../../runtime/manifest.js';
import {assert} from '../../platform/chai-node.js';
import {StorageKeyRecipeResolver} from '../storage-key-recipe-resolver.js';
import {assertThrowsAsync} from '../../testing/test-util.js';

describe('recipe2plan', () => {
describe('storage-key-recipe-resolver', () => {
it('Resolves mapping a handle from a long running arc into another long running arc', async () => {
it('resolves mapping a handle from a long running arc into another long running arc', async () => {
const manifest = await Manifest.parse(`\
particle Reader
data: reads Thing {name: Text}
Expand All @@ -28,33 +29,134 @@ describe('recipe2plan', () => {
@trigger
launch startup
arcId myArcId
arcId writeArcId
recipe WritingRecipe
thing: create persistent 'my-handle-id'
Writer
data: writes thing
@trigger
launch startup
arcId otherArcId
arcId readArcId
recipe ReadingRecipe
data: map 'my-handle-id'
Reader
data: reads data`);

const resolver = new StorageKeyRecipeResolver(manifest);
for (const it of (await resolver.resolve())) {
for (const it of (await resolver.resolve())) {
assert.isTrue(it.isResolved());
}
});
it('fails to resolve mapping a handle from a short running arc into another short running arc', async () => {
const manifest = await Manifest.parse(`\
particle Reader
data: reads Thing {name: Text}
particle Writer
data: writes Thing {name: Text}
recipe WritingRecipe
thing: create persistent 'my-handle-id'
Writer
data: writes thing
recipe ReadingRecipe
data: map 'my-handle-id'
Reader
data: reads data`);

const resolver = new StorageKeyRecipeResolver(manifest);
await assertThrowsAsync(async () => await resolver.resolve(), Error, 'Handle data mapped to ephemeral handle thing.');
});
it('fails to resolve mapping a handle from a short running arc into a long running arc', async () => {
const manifest = await Manifest.parse(`\
particle Reader
data: reads Thing {name: Text}
particle Writer
data: writes Thing {name: Text}
recipe WritingRecipe
thing: create persistent 'my-handle-id'
Writer
data: writes thing
@trigger
launch startup
arcId readArcId
recipe ReadingRecipe
data: map 'my-handle-id'
Reader
data: reads data`);

const resolver = new StorageKeyRecipeResolver(manifest);
await assertThrowsAsync(async () => await resolver.resolve(), Error, 'Handle data mapped to ephemeral handle thing.');
});
it('resolves mapping a handle from a long running arc into a short running arc', async () => {
const manifest = await Manifest.parse(`\
particle Reader
data: reads Thing {name: Text}
particle Writer
data: writes Thing {name: Text}
@trigger
launch startup
arcId writeArcId
recipe WritingRecipe
thing: create persistent 'my-handle-id'
Writer
data: writes thing
recipe ReadingRecipe
data: map 'my-handle-id'
Reader
data: reads data`);

const resolver = new StorageKeyRecipeResolver(manifest);
for (const it of await resolver.resolve()) {
assert.isTrue(it.isResolved());
}
});
it('Invalid Type: If Reader reads {name: Text, age: Number} it is not valid', async () => {
const manifest = await Manifest.parse(`\
particle Reader
data: reads Thing {name: Text, age: Number}
particle Writer
data: writes Thing {name: Text}
@trigger
launch startup
arcId writeArcId
recipe WritingRecipe
thing: create persistent 'my-handle-id'
Writer
data: writes thing
@trigger
launch startup
arcId readArcId
recipe ReadingRecipe
data: map 'my-handle-id'
Reader
data: reads data`);

const resolver = new StorageKeyRecipeResolver(manifest);
// TODO: specify the correct error to be thrown
await assertThrowsAsync(resolver.resolve);
});
// TODO(alxr): Flush out outlined unit tests
it.skip('Short + Short: If WritingRecipe is short lived, it is not valid', () => {});
it.skip('Short + Long: If WritingRecipe is short lived and Reading is long lived, it is not valid', () => {});
it.skip('Invalid Type: If Reader reads {name: Text, age: Number} it is not valid', () => {});
it.skip('No arc id: If arcId of WritingRecipe is not there, it is not valid', () => {});
it.skip('No handleId: If id of handle in WritingRecipe is not provided, it is not valid', () => {});
it.skip('Ambiguous handle: If there are 2 WritingRecipes creating the same handle, it is not valid', () => {});
it.skip('Ambiguous handle + tag disambiguation: If there are 2 WritingRecipes creating the same handle but with different tags and mapping uses one of the tags, it is valid', () => {});
it.skip('No Handle: If there is no writing handle, it is not valid', () => {});
it.skip('No arc id: If arcId of WritingRecipe is not there, it is not valid', () => {
});
it.skip('No handleId: If id of handle in WritingRecipe is not provided, it is not valid', () => {
});
it.skip('Ambiguous handle: If there are 2 WritingRecipes creating the same handle, it is not valid', () => {
});
it.skip('Ambiguous handle + tag disambiguation: If there are 2 WritingRecipes creating the same handle but with different tags and mapping uses one of the tags, it is valid', () => {
});
it.skip('No Handle: If there is no writing handle, it is not valid', () => {
});
});
});

0 comments on commit 337b7b2

Please sign in to comment.