Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

move prepare-test-env-ava helper to a separate package? #6703

Open
warner opened this issue Dec 21, 2022 · 2 comments
Open

move prepare-test-env-ava helper to a separate package? #6703

warner opened this issue Dec 21, 2022 · 2 comments
Assignees
Labels
enhancement New feature or request SwingSet package: SwingSet test tooling repo-wide infrastructure vaults_triage DO NOT USE

Comments

@warner
Copy link
Member

warner commented Dec 21, 2022

What is the Problem Being Solved?

#6561 (comment) is a discussion/lament that a package like swing-store, which wants to run unit tests (using both AVA and SES), needs to import a helper tool:

import { test } from '@agoric/swingset-vat/tools/prepare-test-env-ava.js';

which is exported by SwingSet, but at the same time SwingSet is importing libraries from swing-store. This isn't exactly a circular dependency, because prepare-test-env-ava.js is only used for unit tests, so swing-store has a devDependencies: pointing at SwingSet, whereas SwingSet has a full dependencies: pointing at swing-store. But it's still annoying.

Most of our code uses harden, which is provided as a global when you prepare a SES environment. The easiest way to do this is with import '@endo/init.js'. For unit tests, we frequently want a different set of tradeoffs ("sacrificing safety to optimize for debugging and testing", as SwingSet/tools/install-ses-debug.js says), for which import '@endo/init/debug.js' is the way to go.

Many of our unit tests could do:

import '@endo/init/debug.js';
import test from 'ava';

except(?) AVA has some incompatibilities with SES (maybe changes to Error objects or stack traces?). We have code that works around these in @endo/ses-ava/exported.js. The @agoric/swingset-vat/prepare-test-env-ava.js tool does the following to prepare this:

import '@endo/init/pre-bundle-source.js';
import '@endo/ses-ava/exported.js';
// import './prepare-test-env.js'; // for VatData, see below
import { wrapTest } from '@endo/ses-ava';
import rawTest from 'ava';
export const test = wrapTest(rawTest);

A subset of our code also uses "VatData", which includes things like defineKind and makeScalarBigMapStore. This includes both libraries that use it (e.g. ERTP), and other code or libraries that don't use it directly but import from those libraries.

The core VatData utilities are added to a vat environment by SwingSet (specifically liveslots), as globalThis.VatData. This is wrapped by the @agoric/vat-data package, so userspace code (libraries which need virtual/durable objects, or collections) can do import { defineKind } from '@agoric/vat-data'. But the Compartment in which this is run must either be created by liveslots (running under a real kernel), or it must be set up specially with "fake virtual stuff".

The @agoric/swingset-vat/tools/prepare-test-env-ava also imports prepare-test-env.js, which assembles the cadre of "fake virtual stuff" to support the use of globalThis.VatData outside of a real (liveslots-hosted) vat environment. The tricky part is that

The Problem

We have an awkward split between SwingSet (which defines the kernel, including the liveslots layer that hosts vats), the VatData package (which is imported by userspace, but relies upon a global populated by liveslots), SES (@endo/init), and the tooling necessary to run our unit-test framework of choice (AVA).

SwingSet should not really be hosting AVA-specific tools: swingset uses AVA for its own unit tests, like everything in agoric-sdk, but it doesn't need AVA to run a kernel. And this tool is accessed through a deep import (@agoric/swingset-vat/tools/prepare-test-env-ava.js), which is evil and causes too much coupling between packages (Swingset can't rearrange its internal directory structure without breaking downstream importers).

Description of the Design

I'm not sure yet, but I'm thinking that these tools should move out to their own package(s).

We might make one package specifically for using both AVA and SES at the same time. This would really want to live in the Endo repo. It would need to do the wrapTest() dance listed above.

We might make a second package specifically for using AVA, SES, and VatData at the same time. This would want to live in agoric-sdk, but not in packages/SwingSet/. It needs access to the liveslots tools (virtualObjectManager.js, virtualReferences.js, watchedPromises.js), which normally live in packages/SwingSet/src/liveslots/, but which will probably move to their own package as part of the #6596 effort (we should consider the needs of this tool when we design the exports of the new liveslots package).

Security Considerations

Probably none, this all affects unit tests and packaging, not runtime authorities.

Test Plan

@warner warner added enhancement New feature or request SwingSet package: SwingSet tooling repo-wide infrastructure test labels Dec 21, 2022
@gibson042
Copy link
Member

We should also strive to get rid of prepare-test-env-ava; it can be detrimental and is apparently often unnecessary (cf. #6502 (comment) and the resulting e4bb647).

@ivanlei ivanlei added the vaults_triage DO NOT USE label Jan 4, 2023
@warner
Copy link
Member Author

warner commented Jan 31, 2023

@gibson042 Please take a look at #6863 and see what can be done on top of that branch, to minimize the consequences if we move packages/swingset-vat or packages/liveslots around later (possibly to a separate repository altogether).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request SwingSet package: SwingSet test tooling repo-wide infrastructure vaults_triage DO NOT USE
Projects
None yet
Development

No branches or pull requests

3 participants