I don't think this is actually ready to merge yet. I think I may have written a bug where the body of the request doesn't actually make it into the Request object properly. I'm not yet sure whether this is really a bug or if I've just written my downstream testing code wrong. Please review, but don't merge until I figure this out.

Here's a minimal failing example, which I've taken from the MockRequest doctest and modified slightly:

use tiny_http::{Method, MockRequest, Request, Response, Server, StatusCode};
use std::io::Cursor;
let mut request = MockRequest::new()
    .with_method(Method::Post)
    .with_path("/api/widgets")
    .with_body("42");
struct TestServer {
    listener: Server,
}
let server = TestServer {
    listener: Server::http("0.0.0.0:0").unwrap(),
};
impl TestServer {
    fn handle_request(&self, mut request: Request) -> Response<Cursor<Vec<u8>>> {
        let mut request_payload = String::new();
        request
            .as_reader()
            .read_to_string(&mut request_payload)
            .unwrap();
        assert_eq!(request_payload, "42");
        Response::from_string("test")
    }
}
let response = server.handle_request(request.into());
assert_eq!(response.status_code(), StatusCode(200));

The request_payload should be "42", since that's the string I passed into with_body(), but instead I get the empty string:

thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `""`,
 right: `"42"`', src/request.rs:23:9

Notably, I see the string—and even the byte array—inside From<MockRequest>, so I know it's getting that far. Something about new_request(), as_reader(), or my actual call to new_request() in From<MockRequest> is clobbering the body.

I'm now somewhat suspicious of Request::as_reader(). All of the tests where it appears use TcpStreams, so I wonder if there's some nuance to using simple &[u8]s that isn't accounted for. It's also quite possible that I have an incomplete understanding or am missing something.

@bradfier Any ideas?

Believe I fixed the issue. By default, MockRequest doesn't have any headers. The problem is that new_request checks for nonzero Content-Length and assumes the request is empty if that header is missing. By setting Content-Length in From<MockRequest> if it's missing, we convince new_request to give good output. Note that this approach still allows users to deliberately set Content-Length to interesting values for testing, but the value is correct by default.

I'm getting results that are more like what I'd expect from my downstream code, so I think this is ready for consideration again.

I think this is pretty great, thanks for putting in the effort!

Clippy had one comment where you can replace the if let None = with a postfix .is_none().

What do you think about putting this under crate::utils::test rather than putting it at the same level as Request? We could use doc-links in the doc comments for Request to make it easier to find if you think it would be too hidden in there.

What do you think about putting this under crate::utils::test rather than putting it at the same level as Request? We could use doc-links in the doc comments for Request to make it easier to find if you think it would be too hidden in there.

I think this is a good idea, except for two problems I've found after trying to actually implement it just now. The first problem is that crate::util already has a module called test, which contains one unit test for parse_header_value(). We can solve this easily by giving our new submodule a different name, like testing.

The other problem is that crate::util is private. This causes my existing doctests to fail, and of course it makes external testing impossible. I verified that this is the problem by add pub to mod util and mod testing where appropriate, and the doctests passed without complaint.

I expect that making the entire util submodule public isn't a solution you'd agree with, and to be honest I wouldn't support it either. Other than leaving MockRequest where it is now in crate::request, what options do we have here?

This would be so easily solved if #[cfg(test)] cascaded into dependencies!

I had a look around and if we take Actix as inspiration I think we can just add a src/test.rs module and pop MockRequest in there, made public.

Bikeshedding a bit, I can't decide if MockRequest is appropriate vs TestRequest, there's not really anything Mock about it, it's a real request and nothing is stubbed out, we just construct it manually!

This would be so easily solved if #[cfg(test)] cascaded into dependencies!

I didn't understand what you meant when you mentioned this earlier, but this problem bit me in an unrelated situation the other day, and now I agree that this is an issue.

Anyway, I renamed MockRequest to TestRequest (more descriptive, I think), moved TestRequest to src/test.rs, made it public, and added doclinks to Request.

These changes work for me, so unless you've got other concerns I think this is ready to merge.

I realised there was no good excuse to push such a minor change off onto you, thanks again for the patch!