search

endojs / endo

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

refactor(env-options): migrate test to reduce cyclic dependencies #1896

Closed erights closed 9 months ago

erights commented 9 months ago

closes: #XXXX refs: #XXXX

Description

This PR is non-urgent, and so should wait for the current endo-release-agoric_sdk-upgrade dance to settle down.

Currently, before this PR, we get the following cyclic dependency warnings from yarn build

lerna WARN ECYCLE Dependency cycles detected, you should fix these!
lerna WARN ECYCLE @endo/compartment-mapper -> ses -> @endo/env-options -> @endo/init -> @endo/compartment-mapper
lerna WARN ECYCLE @endo/eventual-send -> (nested cycle: @endo/compartment-mapper -> ses -> @endo/env-options -> @endo/init -> @endo/compartment-mapper) -> @endo/eventual-send
lerna WARN ECYCLE @endo/lockdown -> (nested cycle: @endo/eventual-send -> (nested cycle: @endo/compartment-mapper -> ses -> @endo/env-options -> @endo/init -> @endo/compartment-mapper) -> @endo/eventual-send) -> @endo/lockdown
lerna WARN ECYCLE @endo/promise-kit -> (nested cycle: @endo/lockdown -> (nested cycle: @endo/eventual-send -> (nested cycle: @endo/compartment-mapper -> ses -> @endo/env-options -> @endo/init -> @endo/compartment-mapper) -> @endo/eventual-send) -> @endo/lockdown) -> @endo/promise-kit
lerna WARN ECYCLE @endo/ses-ava -> (nested cycle: @endo/promise-kit -> (nested cycle: @endo/lockdown -> (nested cycle: @endo/eventual-send -> (nested cycle: @endo/compartment-mapper -> ses -> @endo/env-options -> @endo/init -> @endo/compartment-mapper) -> @endo/eventual-send) -> @endo/lockdown) -> @endo/promise-kit) -> @endo/ses-ava
lerna WARN ECYCLE @endo/static-module-record -> (nested cycle: @endo/ses-ava -> (nested cycle: @endo/promise-kit -> (nested cycle: @endo/lockdown -> (nested cycle: @endo/eventual-send -> (nested cycle: @endo/compartment-mapper -> ses -> @endo/env-options -> @endo/init -> @endo/compartment-mapper) -> @endo/eventual-send) -> @endo/lockdown) -> @endo/promise-kit) -> @endo/ses-ava) -> @endo/static-module-record
lerna WARN ECYCLE (nested cycle: @endo/static-module-record -> (nested cycle: @endo/ses-ava -> (nested cycle: @endo/promise-kit -> (nested cycle: @endo/lockdown -> (nested cycle: @endo/eventual-send -> (nested cycle: @endo/compartment-mapper -> ses -> @endo/env-options -> @endo/init -> @endo/compartment-mapper) -> @endo/eventual-send) -> @endo/lockdown) -> @endo/promise-kit) -> @endo/ses-ava) -> @endo/static-module-record) -> (nested cycle: @endo/static-module-record -> (nested cycle: @endo/ses-ava -> (nested cycle: @endo/promise-kit -> (nested cycle: @endo/lockdown -> (nested cycle: @endo/eventual-send -> (nested cycle: @endo/compartment-mapper -> ses -> @endo/env-options -> @endo/init -> @endo/compartment-mapper) -> @endo/eventual-send) -> @endo/lockdown) -> @endo/promise-kit) -> @endo/ses-ava) -> @endo/static-module-record)

The @endo/env-option package participates in many of these, due only to a devDependency in support of a test. This PR moves that test to @endo/ses-ava to reduce cyclic dependencies.

Even with the PR, we still get the following cyclic dependency warnings:

lerna WARN ECYCLE Dependency cycles detected, you should fix these!
lerna WARN ECYCLE @endo/compartment-mapper -> ses -> @endo/compartment-mapper
lerna WARN ECYCLE @endo/lockdown -> (nested cycle: @endo/compartment-mapper -> ses -> @endo/compartment-mapper) -> @endo/static-module-record -> @endo/ses-ava -> @endo/lockdown
lerna WARN ECYCLE (nested cycle: @endo/lockdown -> (nested cycle: @endo/compartment-mapper -> ses -> @endo/compartment-mapper) -> @endo/static-module-record -> @endo/ses-ava -> @endo/lockdown) -> (nested cycle: @endo/lockdown -> (nested cycle: @endo/compartment-mapper -> ses -> @endo/compartment-mapper) -> @endo/static-module-record -> @endo/ses-ava -> @endo/lockdown)

These seem also to be cycles through devDependencies, and so should be fixable by the same means (and with the same costs). However, at least one supports usage from a scripts/*.js rather than a test/*.js . Could these be migrated too? What else would need to be adjusted?

Security Considerations

None

Scaling Considerations

None

Documentation Considerations

It is weird that the tests of a package are not in that package. As we repair other cyclic dependencies, we're likely to cause more of these surprises.

Testing Considerations

After this PR, no tests remain in @endo/env-options . However, the normal yarn test definition is not tolerant of that, so this PR also changes it to "exit 0". However, this means that if tests are added in the future, they may silently be ignored.

Reviewers, to avoid this problem, should this PR instead leave yarn test alone and add a placeholder test instead?

Upgrade Considerations

None