endojs / endo

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

test(ses-ava): use causal console #2109

Closed erights closed 6 months ago

erights commented 6 months ago

closes: https://github.com/endojs/endo/issues/611 closes: https://github.com/endojs/endo/issues/891 refs: https://github.com/Agoric/agoric-sdk/pull/8965#discussion_r1501192561 https://github.com/endojs/endo/issues/647 https://github.com/endojs/endo/issues/1467 https://github.com/endojs/endo/pull/2107 https://github.com/Agoric/agoric-sdk/issues/8662 https://github.com/endojs/endo/issues/1798 https://github.com/endojs/endo/pull/2101 https://github.com/endojs/endo/pull/2107 https://github.com/Agoric/agoric-sdk/wiki/agoric-sdk-unit-testing#patterns

Description

Replaces https://github.com/endojs/endo/pull/2101 and https://github.com/endojs/endo/pull/2107 , though unclosed conversations there probably apply here as well.

Alter ses-ava so that

To get access to the SES's causal console logic, this PR also adds a new export for a privileged power, which ses-ava needs and imports using

import { makeCausalConsoleFromLogger } from 'ses/console-tools.js';

https://github.com/endojs/endo/pull/2107 review comments raise the issue of whether such special powers should be "exported" from ses and "imported" by privileged clients vs import, as this PR currently does, or as a global binding of the start compartment. Unlike https://github.com/endojs/endo/pull/2101 and https://github.com/endojs/endo/pull/2107, this PR currently bundles the ses changes and the ses-ava changes together so we can coordinate changing our minds on this. Reviewers, once that settles, we can split this up into two PRs again if you wish.

Security Considerations

As of this PR, ses-ava will import makeCausalConsoleFromLogger from console-tools.js. We must ensure that this arrangement does not accidentally make these powers available to non-privileged code executing in constructed (non-start) compartments.

Scaling Considerations

none

Documentation Considerations

none

Testing Considerations

The whole point. Since this PR changes only the logging of errors or rejections from the t => {...} function, it will not by itself clean up console output in general. But at least it will cause these error reports to appear at the "right" place in the overall console output.

In addition, the "export" of makeCausalConsoleFromLogger from ses/console-tools.js enables other testing tools to build on our causal console the way ses-ava does.

Compatibility Considerations

Mostly none.

If there are programs post-processing our voluminous console output, they might need to change. However,

Upgrade Considerations

Nothing breaking.

erights commented 6 months ago

May want to redo https://github.com/endojs/endo/pull/701#issuecomment-1974066710 as part of this PR. It would be a natural fit.

erights commented 6 months ago

Apologies to my reviewers, but in responding to @kriskowal 's request about how ses makes the special console-making-power available to @endo/ses-ava at this time, I went about making the change in a way that squashed. I hope this does not make further review too difficult. I have not yet responded to the reviewer comments still open above, and will do so from this baseline.

erights commented 6 months ago

Ping. I addressed all reviewer suggestions. PTAL. thanks.