Hi Guys, Just wanted to let you know I finished the changes related to Snap.Test a few minutes ago, did more or less what Greg suggested. I'm really glad with the outcome of this library, there are still some things to pay attention to:
* Documentation I did some documentation, but probably is not the best quality, if someone can help me on this, give me some recommendations on making documentation more valuable (besides the one on the Snap page), that will be highly appreciated. * Cabal dependencies The dependencies I included in the project are better shown in: https://github.com/roman/snap-core/commit/dbe8f18bbf417be74c6fc2cf5207c55fced02b10 Main observation is the regex-posix library that is used for the assertRequestBody function, I did an arbitrary choice there, and I'm not married with the idea of using that library, if you have any other suggestions, please do recommend The product I have so far is in: https://github.com/roman/snap-core/tree/testing Cheers. Roman.- 2011/3/3 Román González <[email protected]> > > > 2011/3/3 Gregory Collins <[email protected]> > >> Hi Román, >> >> A couple of suggestions about this interface: >> >> 1) there is currently no nice way of setting the request body using a >> ByteString. Users who want to test a handler which accepts PUT >> requests will want this. >> > > Ok. > > >> >> 2) I think for test functions we can make all of the map types >> [(ByteString, ByteString)] instead of [(ByteString, [ByteString])] -- >> repeated headers are an obscure feature of the HTTP specification >> > > I did it like that initially and changed just to keep it up with the > current Snap API. > I agree with you, so cool. > > >> >> 3) buildBoundary wasn't exactly what I had in mind, the length of the >> boundary is not really supposed to vary with the size of the input; >> when I brought up base16 encoding it was in the context of generating >> a fixed-size randomly-generated string. You'd be just as well off >> setting the boundary to a fixed string like >> "___snap_test_boundary___". >> > > I just wanted to do a random boundary, to make it more real. But yeah, that > should be enough, should I do the same with sub-boundaries in file entries > in the multipart? > something like "____snap_test_subboundary___"? > > >> >> 4) looking at the functions exported (get, put, post, get', put', >> post', etc) I see a lot of duplication, or rather a lot of functions >> which do multiple unrelated things together; this is a sign we have >> the model slightly wrong. (For instance, "get" both builds the request >> and runs the test handler.) >> >> I like the idea of building up the Request to test using a monad; I >> think we might be better off if we changed to: >> >> newtype RequestBuilder a = RequestBuilder (State Request a) >> deriving (Monad, MonadState Request) >> >> We could kill a lot of functions if we did this. For instance, instead >> of get/put/post/get'/etc returning the Response, you could have a >> single function: >> >> runHandler :: Snap () -> RequestBuilder () -> IO Response >> >> and you'd have to expand the set of operations in the RequestBuilder >> monad: >> >> setURI :: ByteString -> RequestBuilder () >> setMethod :: Method -> RequestBuilder () >> addHeader :: ByteString -> ByteString -> RequestBuilder () >> postUrlEncoded :: Params -> RequestBuilder () >> postMultipart :: Params -> FileParams -> RequestBuilder () >> setRequestBody :: ByteString -> RequestBuilder () >> >> You could still expose convenience functions for common cases, but >> they would work in RequestBuilder: >> >> get :: ByteString -> [(ByteString, ByteString)] -> RequestBuilder () >> >> so that the use case becomes: >> >> resp <- runHandler foo $ do >> get "/foo" [("bar", "baz")] >> addHeader "X-Forwarded-For" "127.0.0.1" >> >> Let me know what you think. >> > > 4.a) I like the idea of having just one function to get (IO Response), and > makes more sense to makes everything inside the monad, that way you can have > really specialized request buildings done by developers depending on their > needs. > > 4.b) RequestBuilder was initially a State monad of the > Snap.Http.Internal.Request type, however I changed it to make it easier to > build the body of the request from the parameters. > > Having the Request directly makes it really difficult to create the body of > the request because you are receiving parameters one by one instead of the > whole thing. > > Probably the encoding of the body request and adding it to the request can > be done in: > > - postUrlEncoded (should be default to post I guess) > - postMultipart > > And the QueryString encoded in the URI can be added in the modifier: > > - get > > > I will be working on that, I can't promise to have it soonish given that I > had a lot of things to do regarding the Haskell Meetup (next Tues), once > that's done I'll be starting with this. You are not in a hurry right? > > Thanks for the feedback. > > Roman.- > > > > >> >> G >> >> 2011/3/3 Román González <[email protected]>: >> > Guys, >> > I think I'm more or less finished on the first version of the test Snap >> API, >> > we are supporting requests for each HTTP method, requests with file >> uploads, >> > some assertion functions. More assert functions and modifiers should be >> > added when needed. There are also some basic request modifiers, again, >> add >> > more when needed as well (the lazy way) :-) >> > Here is the Snap.Internal.Test file that has both public and private >> > functions: >> > >> https://github.com/roman/snap-core/blob/testing/src/Snap/Internal/Test.hs >> > The Snap.Test that will have the functions we want to export: >> > https://github.com/roman/snap-core/blob/testing/src/Snap/Test.hs >> > And the test done to this API on the Snap test folder: >> > >> https://github.com/roman/snap-core/blob/testing/test/suite/Snap/Test/Tests.hs >> > If you (all the Snap team) have any suggestions/questions regarding >> > documentation, code style or pending functions that you think are really >> > necessary, please let me know. >> > Regards. >> > Roman.- >> > >> > 2011/2/27 Gregory Collins <[email protected]> >> >> >> >> 2011/2/26 Román González <[email protected]>: >> >> > Hey Greg, >> >> > Thanks for the feedback, just one more question (bellow) about >> reading >> >> > the >> >> > files the tester is providing... >> >> >> >> >> > - Reading the contents of the files given by the developer on the >> >> >> > Test >> >> >> > API >> >> >> >> >> >> I'm not sure I follow you here: what specifically is the problem? >> >> > >> >> > I think I'm wondering where are we going to read the files, would we >> be >> >> > requiring the tester to provide a full URI for the File location? >> >> > get' "/my-upload" [] $ do >> >> > addFile "photo" "/tmp/file.jpg" >> >> > Or are we going to have like a SNAP_ROOT where all the paths will go >> >> > get' "/my-upload" [] $ do >> >> > addFile "photo" "tmp/file.jpg" -- this go to >> SNAP_ROOT/tmp/file.jpg >> >> >> >> I would say, for the testing interface, just supply them as >> bytestrings. >> >> >> >> G >> >> -- >> >> Gregory Collins <[email protected]> >> > >> > >> >> >> >> -- >> Gregory Collins <[email protected]> >> > >
_______________________________________________ Snap mailing list [email protected] http://mailman-mail5.webfaction.com/listinfo/snap
