nealrichardson / httptest

A Test Environment for HTTP Requests in R
https://enpiar.com/r/httptest/
Other
79 stars 10 forks source link

Version of with_mock_API that will not assume 200 status #5

Closed byapparov closed 7 years ago

byapparov commented 7 years ago

It will help to extend or add new mock API function that can support codes like 201/202 or error codes.

It is quite normal for data API to return 201 (Created) code, so developer is responsible to request for data again after a period of time.

it will also help to build tests for cases when API returns error, e.g. 403 (Forbidden)

If you have any suggestion on where and how you would like it to be addressed, I will try to add this functionality.

nealrichardson commented 7 years ago

Agree that this would be useful. My first instinct is to look for a .R file if the json payload fixture file isn't found, like:

mockRequest <- function (req, handle, refresh) {
    ## If there's a query, then req$url has been through build_url(parse_url())
    ## so it has grown a ":///" prefix. Prune that.
    req$url <- sub("^:///", "", req$url)
    f <- buildMockURL(req$url)
    ## 1) Look for mock response payload file
    if (file.exists(f)) {
        return(fakeResponse(req$url, req$method,
            content=readBin(f, "raw", 4096*32), ## Assumes mock is under 128K
            status_code=200, headers=list(`Content-Type`="application/json")))
            ## TODO: don't assume content-type
    }
    ## 2) Else, look for .R file with a complete "response" object
    Rfile <- sub("\\..+?$", ".R", f)
    if (file.exists(Rfile)) {
        return(source(Rfile)$value)
    }
    ## 3) Else, stop
    ## For ease of debugging if a file isn't found
    req$url <- paste0(req$url, " (", f, ")")
    stopRequest(req)
}

where the .R file could contain anything, but most usefully it might call fakeResponse with whatever status code, headers, content, etc. you want, or even directly construct a "response" object with the right shape.

Not sure if it's the best or cleanest approach, but it's relatively simple and flexible, so it's where I'd start and see how it goes.

byapparov commented 7 years ago

I really like to "inject" things via closures, here is how I would do it:

https://github.com/byapparov/httptest/pull/3

I have started looking at content type of response as well, and I am not sure if Content-Type or content-type should be used:

a <- GET("www.google.com")
a$headers$`content-type`
# [1] "text/html; charset=ISO-8859-1"
nealrichardson commented 7 years ago

I'm not crazy about that approach because it forces every request within that with_mock_API to respond with the same status code, headers, etc. Suppose I have a function like this:

newEntity <- function (...) {
    resp <- POST("http://somethingsomething.org/api/collection/", 
        body=toJSON(list(...)))
    return(GET(resp$headers$Location))
}

I POST to create an object on the server, and the POST returns 201 with the new object's URL in a Location header. I then want to GET that. That GET will return 200 with some content. I don't want the GET to return 201 with a Location header. And because I'm testing this higher-level function, I can't wrap each of those requests in a different mock context.

This is why I'd prefer to have custom responses per request.

As for content-type vs. Content-Type, HTTP header names are case insensitive according to the RFC.

byapparov commented 7 years ago

Ah, this is an interesting observation.

As this is a unit testing tool, it might force people to separate such function in two. I don't see why it is a good idea to wrap two rest calls into one function.

Once you are ready to test a higher level function you can mock your wrapper functions with with_mock, there is no need to have access to fakes at this point.

The only edge case I see is where you call GET and get 201, then you need to call it again till you get 200. This is actually something that is likely to be a frequent use case. Possibly again worth putting it into a separate issue.

byapparov commented 7 years ago

As some APIs have very long URIs I would even go one step further and allow to inject file paths to the with_mock_API, e.g.:

with_mock_API <- function (expr, status_code = 200, 
                                                        content_type = "application/json",
                                                        response_file = NULL) {
}

This would massively simplify the development without reducing tests quality.

byapparov commented 7 years ago

I have found another way which allows to test for error code behaviour using mockery package.

As an example, I am doing something like this:

context("Error Codes")
library(mockery)

with_mock(
  status_code = mock(401, cycle = T),
  buildMockURL = mock("test.com/401.json", cycle = T),
  {
    with_mock_API({
      expect_error(res <- getReportDefinition(810164), regex = "401")
    })
  }
)

It seems to work, so potentially that is a solution.

byapparov commented 7 years ago

Here is an example of the 201-201-200 test:

library(mockery)
with_mock( # Test that data is extracted correctly from a large report
  status_code = mock(201, 201, 200),
  buildMockURL = mock("test.com/report-data-201.json",
                      "test.com/report-data-201.json",
                      "test.com/report-data-200.csv"),
  http_type = mock("application/json", "application/json", "text/csv"),
  {
    with_mock_API({
       expect_output(res <- getReportData("test.com/dummy", 0.012), regexp = "0.012")
       expect_true(is.data.table(res))
       expect_identical(nrow(res), 2L)
    })
  }
)

I think that is the approach I was looking for, so there is no need to do this in httptest.