Skip to content

Re-enable HTTP and Redis tests#132

Merged
itowlson merged 2 commits intospinframework:mainfrom
itowlson:re-enable-tests
Apr 14, 2026
Merged

Re-enable HTTP and Redis tests#132
itowlson merged 2 commits intospinframework:mainfrom
itowlson:re-enable-tests

Conversation

@itowlson
Copy link
Copy Markdown
Contributor

@itowlson itowlson commented Apr 9, 2026

@fibonacci1729 Right now, this fails because the test response body appears empty (when it is expected to be a "Hello world" message). I can't see what I need to change though - my guess is that something needs to flush or wait longer or something, but I thought collect() would do that for me. Would you mind giving it a once-over to see where my understanding has gone awry please? Thanks (and sorry for loading this on your plate)!

Re-enables HTTP and Redis tests. Thank you Brian and Joel for your help!

@fibonacci1729
Copy link
Copy Markdown
Contributor

I think the root cause is that into_http() and body collection are happening outside run_concurrent(). It seems like the component model's async stream-based body delivery only works while the store is actively running concurrent code. Once run_concurrent returns, the internal stream channels stop being driven, so the body appears empty.

This seems to work:

    let (status, resp_body) = store
        .run_concurrent(async |store| {
            let (res, ()) = tokio::try_join!(
                async {
                    let res = http_service.handle(store, request).await??;
                    let res = store.with(|store| res.into_http(store, async { Ok(()) }))?;
                    let status = res.status();
                    let body = res.into_body().collect().await?;
                    anyhow::Ok((status, body.to_bytes()))
                },
                async {
                    io.await.map_err(|e| anyhow::anyhow!("{e:?}"))
                }
            )?;
            anyhow::Ok(res)
        })
        .await??;

    assert!(status.is_success());
    assert_eq!(resp_body.as_ref(), b"Hello, world!");

@dicej
Copy link
Copy Markdown
Collaborator

dicej commented Apr 10, 2026

Right, as discussed in spinframework/spin#3454, the AsyncFn passed to Store::run_concurrent must not return until it has everything it needs from the guest, including any stream content to be written by the guest after it returns a value. Once that AsyncFn returns, Store::run_concurrent will return, and no guest code can run until/unless Store::run_concurrent is run again.

@itowlson
Copy link
Copy Markdown
Contributor Author

Oh that might affect the Redis trigger then - when I did that I just called run_concurrent - it sounds like that would have a bug if a Redis guest spawned a task.

@itowlson
Copy link
Copy Markdown
Contributor Author

Thanks all - I appreciate both the explanation for understanding, and the helpful fix!

Signed-off-by: itowlson <ivan.towlson@fermyon.com>
Signed-off-by: itowlson <ivan.towlson@fermyon.com>
@itowlson itowlson marked this pull request as ready for review April 12, 2026 22:16
@itowlson itowlson changed the title Re-enable HTTP test Re-enable HTTP and Redis tests Apr 12, 2026
@itowlson itowlson enabled auto-merge April 13, 2026 02:23
@itowlson itowlson requested review from dicej and vdice April 13, 2026 22:48
@itowlson itowlson merged commit 7220808 into spinframework:main Apr 14, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants