@on_viceroy test harness#43
Conversation
f0c0e2e to
14045a3
Compare
9b42c5a to
9964d28
Compare
| try: | ||
| from componentize_py_types import Err # noqa | ||
| except ImportError: | ||
| sys.path.append(str(Path(__file__).parent.parent.parent / "stubs")) |
There was a problem hiding this comment.
This works in tree, but it is a goal of the testing infra we provide in fastly_compute to be distributable and usable by end-customers similar to how we use it in-tree.
There was a problem hiding this comment.
Right. So that's another proposal I have: I think we ought to ship the stubs so customers can run typecheckers against their code. What do you think?
There was a problem hiding this comment.
Yeah, I wasn't sure if we wanted to ship them or to provide the option to generate them (which should be possible). Part of the trouble is where we put them to avoid confusion -- having them on the path when you run the guest code causes problems from my recollection in one failed attempt to move things into tree (though would need to confirm again).
There was a problem hiding this comment.
Since we have the version of componentize-py pinned down, we could generate them deterministically. But I'm not sure how to give customers the option to do so. Some new entrypoint or CLI flag? And then they'd have to arrange for that to be called by their testrunner (or outer build process, if they have one), and we'd have to document it and get them to read. It might be easier for everyone to just ship them as we ship the generated exceptions.
We could stick them any old place; I offer fastly_compute/stubs. It's not a package (having no __init__.py), so nobody will be tempted to import it via fastly_compute.stubs.... And it shouldn't interfere with guest-side imports because nobody put it on the PYTHONPATH. I've got that happening in tests/init.py, which I wouldn't expect guests to import. Here's the reasoning from the commit message:
Import stubs (once and only once) iff we're not under Viceroy. It turns out they are needed if we are to run typechecking on the host. In addition, some tests that can get along perfectly well with only stubs, saving a spin-up of Viceroy. Finally, a universal import of stubs on the host means we don't have to segment modules like utils.py into guest and host halves to avoid import errors.
We might be able to restate that path patch as a package-wide pytest fixture, but I'm not sure it would run early enough to satisfy imports.
| /// Virtual environment in which to look for modules (default: | ||
| /// VIRTUAL_ENV env var or .venv) | ||
| #[arg(short, long)] | ||
| virtualenv: Option<PathBuf>, |
There was a problem hiding this comment.
So I understand, this was needed due to the acrobatics we're doing in the test harness and probably not likely to be needed by users normally (it is not for componentize-py which is the functionality I was largely reproducing).
There was a problem hiding this comment.
Correct. It allows us to build against the venv the testrunner is running under, which elegantly has exactly the packages we need.
…decorator. * Add `@on_viceroy` decorator and supporting `AutoViceroyTestBase` test subclass. These work together to generate ephemeral Fastly projects under which Viceroy-side code is run. * Add `--virtualenv` option to `fastly-compute-py` so we can direct it to the env we're running under, even if it's not called ".venv" or if it's not pointed to by `VIRTUAL_ENV`. * Add Bottle to testing deps because we use it to communicate with the in-Viceroy code. * Move exceptions tests (which I'm using as a PoC) inside the package because their path needs to be importable for the machinery to work. But don't worry: we still don't ship them with the distro. * Remove unused FASTLY_COMPUTE_PY_BIN from makefile. * Import stubs (once and only once) iff we're not under Viceroy. It turns out they are needed if we are to run typechecking on the host. In addition, some tests that can get along perfectly well with only stubs, saving a spin-up of Viceroy. Finally, a universal import of stubs on the host means we don't have to segment modules like utils.py into guest and host halves to avoid import errors. * Add Jinja to config-store's uv.lock, which should have been in 87f875f. It got neglected in the rapid-fire landing of the config-store API and exception polishing.
...for consistency and to allow future use of `@on_viceroy`. We still exclude them when we publish the distribution.
This better reflects what it is: `_server` is initialized when the class is defined. More immediately, it lets `@on_viceroy` call `.request()` without needing to access a private attr.
Previously, we were just sucking in stdin, which isn't hooked up to anything useful.
In ruff's case this solves a bunch of really weird complaints about class methods: ``` ERROR Expected a callable, got `classmethod[Unknown, [**kwargs: Any], Any]` [not-callable] --> fastly_compute/tests/test_exception_remapping.py:82:13 | 82 | self.raise_int_by_surprise() ```
9964d28 to
c2ef65a
Compare
…er) proof of concept.
`del`ing the symbol counts as a use.
posborne
left a comment
There was a problem hiding this comment.
I'm OK moving forward with this as a building block, though I think a few parts of this might end up not being totally scalable:
- The venv / sys.prefix bits might be finnicy in some deployments but it will be fine for our use.
- As discussed, runtime path modifications are still a bit messsy, but I'm fine deferring.
- Needing to compile potentially quite a few more wasms on each test run may start to add up over time. We might be able to parallelize around some of that and make it less of an issue, but we can probably deal with that as it comes. Binary caching based on a hash of the input text might also be workable (though we would need a way to also invalidate that cache), but I wouldn't build that until it becomes a sufficient annoyance.
|
Thanks! (1) (3) Ah, true, we would rebuild rather than using |
Make it short and easy to run certain methods in a Viceroy guest.
Just decorate the method you want in Viceroy, and an ephemeral app is created; dispatch is handled; and serialization of params, return values, and exceptions is done in a standard way.
Advantages:
examplesdirThis also adds support for request bodies in our WSGI façade, which we forgot to do.
This stacks atop #38, so review that first.
For a before and after, see the last commit, where I port the ConfigStore tests.