Add outbound-wasi-http-0.2.0 tests
#2
Conversation
Signed-off-by: Ryan Levick <ryan.levick@fermyon.com>
Signed-off-by: Ryan Levick <ryan.levick@fermyon.com>
| { | ||
| "name": "transfer-encoding", | ||
| "value": "chunked" | ||
| }, |
There was a problem hiding this comment.
@lann I was a bit surprised by this as the guest is just returning a buffer of bytes. I assume something funny is happening in wasi-http's implementation to cause this.
There was a problem hiding this comment.
I think this is expected when no Content-Length is given.
There was a problem hiding this comment.
I was able to confirm that indeed, if a Content-Length header is give, the transfer-encoding header disappears.
rylev
force-pushed
the
package
branch
2 times, most recently
from
a5f616a
to
4c7840b
Compare
Signed-off-by: Ryan Levick <ryan.levick@fermyon.com>
| 'outer: loop { | ||
| 'inner: for captures in regex.captures_iter(content) { |
There was a problem hiding this comment.
This might be less confusing if you break out the iterator, e.g.
| 'outer: loop { | |
| 'inner: for captures in regex.captures_iter(content) { | |
| 'outer: loop { | |
| let mut captures_iter = regex.captures_iter(content); | |
| 'inner: for captures in captures_iter { |
| std::fs::copy(path, test_archive.join(&component_file))?; | ||
| if template_key.trim() == "source" { | ||
| manifest.replace_range(full.range(), &component_file); | ||
| 'outer: loop { |
There was a problem hiding this comment.
Is this the same code as replace_template above?
There was a problem hiding this comment.
Yea, I could certainly make the test-environment a dependency of this crate.
| }, | ||
| { | ||
| "name": "Date", | ||
| "optional": true |
There was a problem hiding this comment.
Does this imply that this header check is exhaustive? I wonder if that's necessary or if we'd be better off just checking for certain headers (perhaps with e.g. null meaning "reject if this header exists").
🤷
There was a problem hiding this comment.
Yes, the header check is exhaustive. I'd like to stick with exhaustive checks for now, but I understand that as we try to document what runtimes are and are not allowed to do we might have to be less restrictive in the future.
There was a problem hiding this comment.
I feel quite confident that runtimes won't return all the same headers, but waiting is fine.
There was a problem hiding this comment.
Actually, I think the transfer-encoding example above is a good one here; it would have been semantically the same to return with no transfer-encoding (under certain circumstances...).
Signed-off-by: Ryan Levick <ryan.levick@fermyon.com>
| "path": "/", | ||
| "headers": [ | ||
| { | ||
| "name": "url", |
There was a problem hiding this comment.
Any reason why we want to set the url in the header rather than as a query parameter or in the body?
There was a problem hiding this comment.
Nope - is there any reason to prefer one over the other?
There was a problem hiding this comment.
Not particularly. I am not used to data being consumed by the service being in the header but to your point, I don't think it matters
There was a problem hiding this comment.
I think headers are appropriate here; they are the most "metadatay" option in HTTP.
| .get(template_value) | ||
| .with_context(|| format!("'{template_value}' is not a known component"))?; | ||
| let component_file = "component.wasm"; | ||
| std::fs::copy(path, test_archive.join(component_file))?; |
There was a problem hiding this comment.
Is this only evaluating the source under component.test? I think we need to update the request-shape app's spin.toml. Right now, i believe that the source file is not being replaced and the shim tests are failing with:
thread 'main' panicked at conformance-tests/src/main.rs:38:10:
called `Result::unwrap()` on an `Err` value: failed to copy file '/tmp/conformance-tests/request-shape/component.wasm' to temporary directoryThere was a problem hiding this comment.
The tests work for me in the shim. Are you updating the shim conformance test binary to use the last version of the conformance-tests library?
There was a problem hiding this comment.
I was, but I'll give it another go
|
Going to merge this as I don't want to hold up other PRs. |
This adds two tests that test outbound-http (with permissions and without).
This required a few new features to conformance testing:
Here's a branch updating Spin to use the changes here: https://github.com/fermyon/spin/tree/update-conformance-tests