Stage to master #364
Stage to master #364
Conversation
New and improved server tests
Add write test
Capture errors curl reports on fusedav-valhalla requests
Update tests for curl capture error
Report curl errorbuffer errors via print_errors mechanism
Dev to yolo
Yolo to stage
|
@kf6nux You may have reviewed a few weeks back, but if you have time, can you give it another gander? Thanks. I guess I still have some CircleCI stuff to do as well. |
src/filecache.c
Outdated
| @@ -531,7 +531,7 @@ static void get_fresh_fd(filecache_t *cache, | |||
| // but on the close (dav_flush/release), the PUT fails and the file never makes it to the server. | |||
| // On opening again, the server will deliver this unexpected 404. Changes for forensic-haven | |||
| // should prevent these errors in the future (2013-08-29) | |||
| if (inject_error(filecache_error_fresh404)) response_code = 404; | |||
| if (inject_error(filecache_error_fresh404)) response_code = 400; | |||
There was a problem hiding this comment.
It might be confusing to inject 400s when checking filecache_error_fresh404.
There was a problem hiding this comment.
Made the changes.
tests/server-tests/api_test.go
Outdated
| @@ -82,7 +77,7 @@ func testMethod(t *testing.T, testInput TestInput) error { | |||
|
|
|||
| for _, pair := range testInput.headers { | |||
| req.Header.Add(pair.key, pair.value) | |||
| fmt.Printf("testMethod: method: %s; path: %s; header: %v\n", testInput.method, testInput.path, req.Header) | |||
| fmt.Printf("testMethod: Headers: method: %s; path: %s; header: %v\n", testInput.method, testInput.path, req.Header) | |||
There was a problem hiding this comment.
I'm not sure I'm mentally parsing this log line correctly. Why do the labels testMethod: and Headers: not have corresponding fields?
There was a problem hiding this comment.
testMethod is the name of the function. It's a pattern I use in the fusedav code to list the function name in the log message. Not sure the value here. Leaving it in. As for 'Headers:', this is just an information message, and Headers is just further definition of what the message is conveying.
tests/server-tests/api_test.go
Outdated
| if testInput.expectedStatusCode != 404 && | ||
| len(testInput.content) > 0 && | ||
| string(body) != testInput.content { | ||
| t.Errorf("testMethod: Error, expected content %v, got %v", testInput.content, resp.Body) |
There was a problem hiding this comment.
I think you want to pass string(body) as an argument here instead of resp.Body.
tests/server-tests/api_test.go
Outdated
|
|
||
| testInput.client = getClient() | ||
|
|
||
| paths = make([]PathResult, 7) |
There was a problem hiding this comment.
It might read better to use a literal here instead of making and then setting a slice. 💯 for slice/map based tests tho. 😀
There was a problem hiding this comment.
paths[0] is not a constant, and all other values build on it. Not sure how to put together the literal.
tests/server-tests/api_test.go
Outdated
| testInput.expectedStatusCode = entry.statusCode | ||
| err := testMethod(t, testInput) | ||
| if err != nil { | ||
| // handle error |
There was a problem hiding this comment.
I'm not sure if ignoring an error here is going to cause pain in debugging later.
tests/server-tests/api_test.go
Outdated
| @@ -203,16 +257,16 @@ func TestBasicFileOps(t *testing.T) { | |||
| // handle error | |||
| } | |||
|
|
|||
| // Clear the headers | |||
| testInput.headers = nil | |||
|
|
|||
| testInput.method = "GET" | |||
| testInput.expectedStatusCode = 200 | |||
| err = testMethod(t, testInput) | |||
| if err != nil { | |||
| // handle error | |||
There was a problem hiding this comment.
I'm not sure if ignoring an error here is going to cause pain in debugging later.
Add CURLOPT_ERRORBUFFER to capture better error information. However, this may not apply to those '400' errors we were seeing on GET requests, since those are not 'curl errors'.
Also some additions to fusedav tests, as well as expansion of 'server' tests, which are essentially 'externalized' tests to replace python server unit tests.