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

Implement a mock virtual object manager to support unit tests outside SwingSet #2543

Merged
merged 2 commits into from
Mar 1, 2021

Conversation

FUDCo
Copy link
Contributor

@FUDCo FUDCo commented Feb 26, 2021

In support of #2534, implements a fake version of the SwingSet virtual object manager for unit tests that need to run outside the SwingSet environment.

Provides a function makeFakeVirtualObjectManager(optionalCacheSize) that returns a set of the virtual object functions that act like the real thing but which store stuff in an ephemeral in-memory map and don't pull in the rest of the SwingSet environment. They do, however, do this via the actual SwingSet kernel virtual object manager code -- what we're faking out is the store and the functions given to the marshaler.

Usage:

import { makeFakeVirtualObjectManager } from '@agoric/swingset-vat/tools/fakeVirtualObjectManager';
...
const { makeKind, makeWeakStore } = makeFakeVirtualObjectManager();

(the return object also includes dumpStore and flushCache functions, but you shouldn't normally need to mess with those).

It turned out that the unit tests for the virtual object manager itself already contained 95% of what was needed, so this is mostly a repackaging of that machinery. In order to test this I rewrote the virtual object manager unit tests in terms of it, so I'm reasonably sure the logic is correct.

One thing this code does not do, which was requested in the original ticket #2534, is mess with the global environment. I'm going to guess that it may be sufficiently straightforward for unit test setup code to just do the import, call makeFakeVirtualObjectManager, and make the resulting virtual object functions available to the test code. However, if messing with the global environment is truly called for, then somebody who actually understands whatever dance is required to do that will need to give me some help.

@FUDCo FUDCo added enhancement New feature or request SwingSet package: SwingSet tooling repo-wide infrastructure labels Feb 26, 2021
@FUDCo FUDCo self-assigned this Feb 26, 2021
@katelynsills
Copy link
Contributor

Might be nothing important, but when I was trying to rebase on top of this, I got a ton of merge conflicts. It's also showing up strangely in git visualization:

Screen Shot 2021-02-26 at 4 05 44 PM

@katelynsills katelynsills mentioned this pull request Feb 27, 2021
17 tasks
Copy link
Contributor

@katelynsills katelynsills left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code here looks good to me, but I am seeing what looks like a bug when using this.

I was able to use it in my ERTP AVA tests which is awesome, though! See #2526 (comment) for more details.

Since I'm using #2526 as the test for whether this PR is correct, would it make sense to stack them on top of each other so we can automatically test by running yarn test in packages/ERTP?

Copy link
Member

@warner warner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, glad the unit test was already almost in the shape we needed for this

@warner
Copy link
Member

warner commented Mar 1, 2021

Since I'm using #2526 as the test for whether this PR is correct, would it make sense to stack them on top of each other so we can automatically test by running yarn test in packages/ERTP?

Yeah, probably with this PR as the base (i.e. it remains targeting master), and yours which needs this feature on top (targeting implement-2534). OTOH maybe we just land this one now and figure out fixes in a separate PR. Especially since this needs to be rebased anyways now that trunk has moved on by a few commits.

@FUDCo FUDCo merged commit 9871903 into master Mar 1, 2021
@FUDCo FUDCo deleted the implement-2534 branch March 1, 2021 17:12
@FUDCo
Copy link
Contributor Author

FUDCo commented Mar 1, 2021

closes #2534

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 tooling repo-wide infrastructure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants