Update WIT definitions, add tests & CI infra#7
Merged
Conversation
posborne
force-pushed
the
posborne/update-wit-ci-and-tests
branch
2 times, most recently
from
30919ef to
a279d39
Compare
erikrose
reviewed
Sep 3, 2025
Member
erikrose
left a comment
erikrose
left a comment
There was a problem hiding this comment.
Hooray! Looking great! Just a few notes.
posborne
force-pushed
the
posborne/update-wit-ci-and-tests
branch
from
29f294e to
c239fcc
Compare
Find an available port and use that for a given test run to avoid interference with other running viceroy instances or services on the local machine.
We key off extra output included when running the server in verbose mode to know when viceroy is ready.
A lot of the extra stuff we had for logging exceptions and such aren't required any longer as the WASI bits are working as they ought to, so we can gut all of that. Also, fixed vcpu_time which does return an integer number of milliseconds in the viceroy impl, so assert on that.
posborne
force-pushed
the
posborne/update-wit-ci-and-tests
branch
from
8165059 to
ef3406b
Compare
erikrose
reviewed
Sep 3, 2025
Member
erikrose
left a comment
erikrose
left a comment
There was a problem hiding this comment.
So nice to get rid of that 10s sleep. :-D
We are using these helpers internally and it probably makes sense to have some sort of easier setup for end users. This is the first stab at making that available. Along with those changes, I also incorporated a few chanes to reduce the code for viceroy setup using asyncio (still a bit verbose but is working) and a few other small tweaks. The plugin to get Viceroy output on failure is a hack and we may scrap or have to revisit other approaches (e.g. just immediately outputting to host stdout/stderr) but it is functional and may be a reasonable approach (if ugly in implementation).
posborne
commented
Sep 3, 2025
posborne
force-pushed
the
posborne/update-wit-ci-and-tests
branch
from
5c8041a to
2bc81e9
Compare
We hide the viceroy_server by making it be an autouse fixture on the class which lets us clean things up a fair bit. It stands to reason that all viceroy based tests will want a server.
This was referenced Sep 4, 2025
erikrose
reviewed
Sep 5, 2025
Member
erikrose
left a comment
erikrose
left a comment
There was a problem hiding this comment.
Looking good. I just want to conclude the threads on mark.integration and the lock that I was curious about.
| Returns: | ||
| requests.Response: The HTTP response | ||
| """ | ||
| timeout = kwargs.pop("timeout", self.REQUEST_TIMEOUT) |
Member
There was a problem hiding this comment.
It'd be nice to lift timeout up into the function signature. Not a blocker. More a note to my future self.
erikrose
reviewed
Sep 5, 2025
|
|
||
| ```bash | ||
| make test # Build and run tests | ||
| pytest -m integration # Run integration tests only |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.