Use prepare-test-env and ses-ava in SwingSet where possible

[erights]

Enhance prepare-test-env beyond #2703 to also bundle in ses-ava.

Use it in SwingSet tests where possible. This does not include the metering tests.

Progress on #2662 but does not yet close it.

[erights]

@michaelfig for some reason, it fails in pegasus. It seems related to tape or tap !

[warner]

I think that one duplicate import is the only thing that really needs to change.

The inconsistency of the lint override for import ordering bugs me, it makes me think I don't understand what prettier wants to do. I'd be happiest if none of the files had an override. I'd be second happiest if all of the files had an override. If the override is really only necessary in some and not others, I'll be unhappy until I understand what makes a difference.. is it the spelling of the second import and whether it would sort before/after ../../tools/something?

[FUDCo]

This looks pretty good. And many thanks for shouldering the tedium!

Have you looked into what would be involved in making this work with test-message-patterns.js? That's the biggest gap for me so far, if only because that test is currently the primary exercise point for what I've been in the middle of working on (and likely will continue to be for a while yet).

[FUDCo]

(I should add that if it's just a matter of wrapping a few more Ava API entry points in ses-ava, I'm willing to take that on, assuming it's not too deeply weird.)

[erights]

@warner Done. PTAL

@michaelfig please look at the Pegasus error and advise. Thanks.

[michaelfig]

@michaelfig please look at the Pegasus error and advise. Thanks.

Pegasus doesn't use AVA. This failure was caused by a combination of well-intentioned changes: @katelynsills changed Pegasus tests to use prepare-test-env, then you changed prepare-test-env to use AVA. AVA crashes if it is imported in a non-AVA test.

This is either a regression we should deal with some other way, or you can say it's acceptable to force users of prepare-test-env (i.e. every dapp that is trying to emulate our test environment) to adopt AVA. I don't like the latter very much, but I can accept that it if it is intentional.

In my mind, this might be a misstep, and prepare-test-env should work without requiring AVA. Maybe that's not possible.

[michaelfig]

In my mind, this might be a misstep, and prepare-test-env should work without requiring AVA. Maybe that's not possible.

Easiest way to solve this is to create a prepare-test-env that doesn't provide the AVA-ised test export. Then import that module directly in the Pegasus tests and in the other prepare-test-env-ava.

[michaelfig]

As long as the mechanism is there to fix the Pegasus situation, go ahead and ignore the error and merge. I can apply the fix soon.

[michaelfig]

That makes me happy!

Are there reasons there are some test files that still have:

import 'prepare-test-env-ava';
import test from 'ava';

? If you're trying not to change semantics, that's a good enough reason.

[erights]

That makes me happy!

Likewise! This separation you suggested is definitely nice.

Are there reasons there are some test files that still have:

import 'prepare-test-env-ava';
import test from 'ava';

? If you're trying not to change semantics, that's a good enough reason.

Yup, that's why. In this PR trying to set the stage for that change, but wanted to do that change in a succeeding PR. Coming soon!

[warner]

Looks good. In the long run I'm slightly nervous about encouraging the deep import (with tools/ in the name), which might impose some constraints on how we organize swingset's internals. OTOH I think there's a mechanism in package.json to make arbitrary mappings from the name in the import to the location of the file within the package directory, so if we want to rearrange files, we could use that mechanism to keep the external name stable. The pattern we're thus establishing is that @agoric/swingset-vat/X is the actual vat/controller/VM facility, and @agoric/swingset-vat/tools/Y is where you get tools to run tests that exercise code which wants to run in a swingset vat environment. Which seems fine to me.

[erights]

Looks good. In the long run I'm slightly nervous about encouraging the deep import (with tools/ in the name), which might impose some constraints on how we organize swingset's internals. OTOH I think there's a mechanism in package.json to make arbitrary mappings from the name in the import to the location of the file within the package directory, so if we want to rearrange files, we could use that mechanism to keep the external name stable. The pattern we're thus establishing is that @agoric/swingset-vat/X is the actual vat/controller/VM facility, and @agoric/swingset-vat/tools/Y is where you get tools to run tests that exercise code which wants to run in a swingset vat environment. Which seems fine to me.

This sound good. But this PR made no progress towards that.