read test via telnet-host
#337
Merged
Conversation
This file contains 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
This makes failure messages marginally less useful, but it aligns `moot` with how MOO *actually* works. This refactor makes it possible to express more complex flows.
abesto
commented
Jun 24, 2024
abesto
commented
Jun 24, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
I accumulated a bunch of biggish changes while working on the suspend/read/notify tests. It'd be good to land them, with the as-yet-failing test disabled, to avoid a long-running branch with big changes.
Refactor
Before this PR, the execution model of
mootis incompatible with flows involvingread(). Changed things up to fix that; also added docs about the syntax of.mootfiles which has grown beyond trivial by now.Extended
.mootsyntax:=exact output"Normal" assertions in
.mootfiles represent MOO values (i.e.Vars). In practice, this means we run the expected value through an extraevalto resolve them. This allows nice things like usingplayeras expected output, and getting the comparison to work as we'd naively expect.Notably, this is the only way we can even do assertions at all when running
Scheduler-based tests, because fromSchedulerwe get aVaras the result of theevalcall (and not some string-serialized response).This flow becomes suuuper fragile in the presence of
read(), because the order of messages is hard to reason about (and maybe not even guaranteed). So instead of trying to figure that out, let's introduce a way to say: do not do any kind of extra processing; take the expected value as a string, and compare it to our actual result as a string. This only works on network-based tests.(I guess we could Be Smart, and do the extra resolve step when running with Scheduler and not do it when running via the network, but it feels like being explicit here is the less confusing approach)
Failing
suspend()+read()testWeirdly, the
; notify(player, "read: " + read()); return "retval";case seems to be passing consistently now. Probably #338 made a bigger impact than I initially thought.Starting this with
suspend(0.1)makes it fail though; that test is currently commented out.