endojs / endo

Endo is a distributed secure JavaScript sandbox, based on SES
Apache License 2.0
804 stars 71 forks source link

feat(ses-ava): import test from @endo/ses-ava/prepare-endo.js #2133

Closed erights closed 6 months ago

erights commented 6 months ago

closes: #XXXX refs: #XXXX

Description

At https://github.com/Agoric/agoric-sdk/pull/9026#discussion_r1511360711 @turadg suggested that rather than scatter redundant prepare-test-env-ava.js test files in each test directory, that @endo/ses-ava should just export a test that most ses-ava users can just use directly. This PR attempts to provide that.

To do so, I had to change the layering so that the new @endo/ses-ava/prepare-test-env-ava.js can do the endo initialization that most clients expect, which includes setting up both ses and @endo/eventual-send with various options set up appropriately for debugging. Thus, for all those packages that @endo/ses-ava now depends on, directly or indirectly, I changed them to use 'ava' rather than @endo/ses-ava to avoid circular build dependencies.

I also had to move @endo/ses-ava's dependency on 'ava' from "devDependencies": "dependencies":. This creates a hazard that bundlers might try to bundle 'ava', which would be bad. To avoid that, all packages using @endo/ses-ava must depend on it as a "devDependencies":, which they should do anyway.

From a pair debugging session with @Chris-Hibbert just now, I think the options set up by this new @endo/ses-ava/prepare-test-env-ava.js are adequate for him to get the stack traces he needed.

Security Considerations

none

Scaling Considerations

none

Documentation Considerations

easier to document how to set up tests in a test directory.

Testing Considerations

The point. This should both make it easier to set up a test directory, and to get more options set right to support debugging.

Compatibility Considerations

This PR mades a coordinated change between various endo packages, without trying to tolerate version slippage among them. This is all in preparation for a new endo release, and effects on tests, so should be fine. Should have no compat impact on any existing clients of anything in endo.

Upgrade Considerations

none

Nothing breaking. NEWSworthy.

Chris-Hibbert commented 6 months ago

I've looked at the changes. I approve of the goal, but I leave it to others to approve the PR.

erights commented 6 months ago

At https://github.com/Agoric/agoric-sdk/pull/9026#discussion_r1511357998 @turadg suggested removing the unneeded lint suppression. With this change, I was able to remove all unneeded

// eslint-disable-next-line import/order

Good riddance! Done.

erights commented 6 months ago

@turadg , I acted on all suggestions as well as possible and got @kriskowal 's approval. PTAL.

Thanks for the great suggestions! It really is much better now.

gibson042 commented 6 months ago

We had planned,

but I think this is a good solution to the side-effect surprises and boilerplate. @gibson042 should this PR close that issue?

Yes, I would say so. This is great!