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

Provide scaffolding for testing scenario3's home objects #1077

Merged
merged 2 commits into from
May 11, 2020

Conversation

michaelfig
Copy link
Member

Hi @katelynsills, here is the scaffolding I owe you. Feel free either to approve and merge this PR or evolve it in-place to include the testing you want to do.

The idea is in test-home.js, you can introduce many individual tests between the setup and teardown. If you want separate test-*.js files, you can do that, too.

On my machine, it takes about 12 seconds to set up the fixture. YMMV.

@michaelfig michaelfig added the cosmic-swingset package: cosmic-swingset label May 9, 2020
@michaelfig michaelfig requested a review from katelynsills May 9, 2020 03:37
@michaelfig michaelfig self-assigned this May 9, 2020
@michaelfig michaelfig force-pushed the cosmic-test-fixture branch from 3fdcd0b to 19606a6 Compare May 9, 2020 03:53
@michaelfig michaelfig requested a review from warner May 9, 2020 03:55
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.

Clever, and I'm going to approve it, and the makeFixture API is great and I'm happy to see tests written against it.

But.. ouch. Spawning make in a subprocess to launch an additional subprocess glued together by websockets.. that's adding to our complexity budget, not reducing it.

I'd like to see if we can replace makeFixture() with one that runs everything in-process. We need to reduce our use of that Makefile as a coordination tool, I originally added that as a crib sheet because I couldn't remember the CLI commands to run, and it's grown a bit too much since then. We should be able run a swingset in the same process as the test program, perhaps in a separate thread if really necessary, and incorporate all the necessary vats as libraries (the "(de)central services" and client-side services that we've discussed splitting out).

And I might be wrong but I don't think these kinds of tests need a full swingset kernel. The test code is just doing eventual-sends to objects, and then some assertions about the results. We need objects in the same shape as the ones we load into swingset vats, but I don't think we need the vats.

What if we start by changing the swingset config to know about "liveslots vats" (as distinct from "raw vats"), which are defined by a module whose default export is the build function (rather than the setup function of a raw vat).

Then we make a library that builds "(de)central services" out of these functions, in the same way that the chain-side bootstrap vat would, except it's not using vats, it's just using E() to wire the objects together in the same way. This might actually be the same function as our current bootstrap(), except maybe without the devices, that wouldn't be relevant/meaningful in a non-chain test case.

Then this test code calls that library function to obtain the same objects as you'd get from a bunch of vats, but there are no vats involved, just a bunch of objects with the same relationships as before.

Then the test code is calling E() on those objects and awaiting responses. Yes eventual-send but no vats, no kernel, no swingset, no captp, no multiple processes, no Makefile. Debuggable with normal tools, even.

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.

This looks great! Thank you so much for putting this together. I'm all for the future improvements that Brian is mentioning, as long as the test is actually testing bootstrap.js code, and not mocking it.

@michaelfig michaelfig merged commit d981d27 into master May 11, 2020
@michaelfig michaelfig deleted the cosmic-test-fixture branch May 11, 2020 22:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cosmic-swingset package: cosmic-swingset
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants