Modify the test module, add the file upload test, and add the image edit api #63
Conversation
Rascal0814
commented
Feb 9, 2023
Rascal0814
commented
- Because the test files are all written in one file, I think it is unreasonable to make changes
- File upload test
- Add image edit api and corresponding test file
- An adjustment has been made to OpenAITestServer. The test needs to register the handler with the server
|
Thank you for the PR! 🙌🏻 |
image_test.go
Outdated
| return | ||
| } | ||
|
|
||
| mask, err := os.Open("./static/image_edit_mask.png") |
There was a problem hiding this comment.
It's a bit of overkill bringing static images into the repo to test with the mock server
internal/api/server.go
Outdated
| @@ -0,0 +1,43 @@ | |||
| package api | |||
There was a problem hiding this comment.
May I suggest we place it into /test or /testing or /internal/test[ing]? The /internal/api might look like it's not test-related.
See also: https://github.com/golang-standards/project-layout
There was a problem hiding this comment.
Maybe we can even place this file on the root level and make the whole test server stuff non-exportable
There was a problem hiding this comment.
Although internal cannot be exported, in order to simplify the project structure, it is better to put it in the root directory
There was a problem hiding this comment.
Another problem is that the package of the test file is different from that of the normal file. Therefore, if the method is set to not exportable, it cannot be accessed
internal/api/server.go
Outdated
| type Handler func(w http.ResponseWriter, r *http.Request) | ||
|
|
||
| // serverMap. | ||
| var serverMap = make(map[string]Handler) |
There was a problem hiding this comment.
Not sure that global map is a good idea here, maybe we can do something along the lines of:
type TestServer struct {
handlers map[string]Handler
}
func NewTestServer() *TestServer { ... }
func (ts *TestServer) RegisterHandler(path string, handler Handler) { ... }Thanks!
There was a problem hiding this comment.
Should the map in RegisterHandler be determined whether it exists or directly overwritten?
There was a problem hiding this comment.
I think global map is feasible here, and there is no need to consider concurrency. Maybe I think it is simple, don't you think?
There was a problem hiding this comment.
We also have GetTestToken here, I think it makes sense to keep test-server-related logic encapsulated in a single tidy struct, in that way, it would be obvious how to extend the test server in the future.
There was a problem hiding this comment.
Well, I'll make it simple
|
Thank you for all the changes! |
