Add tests & small refactoring of recipedb#895
Add tests & small refactoring of recipedb#895burov wants to merge 2 commits intoGoogleCloudPlatform:masterfrom
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: burov The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| recipes map[string]Recipe | ||
| } | ||
|
|
||
| func newRecipeDB(path string) (*recipeDB, error) { |
There was a problem hiding this comment.
Maybe also take a timeFunc here as an argument? This would result with a clear separation between a factory method that creates from all components and one that uses defaults.
|
|
||
| func newRecipeDB(path string) (*recipeDB, error) { | ||
| db := &recipeDB{ | ||
| file: path, |
|
|
||
| wantRecipes map[string]Recipe | ||
| wantErr error | ||
| }{ |
There was a problem hiding this comment.
maybe also a case with empty DB (but correct json) ?
| return nil | ||
| }, | ||
|
|
||
| wantRecipes: map[string]Recipe{ |
There was a problem hiding this comment.
nit: I wonder if it makes sense to check for content (instead of e.g. a count), if you have a separate cases for checking the content
feel free to ignore
| } | ||
| } | ||
|
|
||
| func Test_recipeDB_saveToFS(t *testing.T) { |
There was a problem hiding this comment.
I wonder if instead of checking the JSON string (and relying on zero changes to serialization code) it might make more sense to re-read the DB from the drive and checking if recipe is there.
| utiltest.EnsureResults(t, false, ok2) | ||
| } | ||
|
|
||
| func Test_recipeDB_addRecipe(t *testing.T) { |
There was a problem hiding this comment.
Maybe re-read DB from drive instead of checking the JSON string?
| if _, err := f.Write(dbBytes); err != nil { | ||
| if _, err := f.Write(raw); err != nil { | ||
| f.Close() | ||
| return err |
There was a problem hiding this comment.
should we also delete the temp file when writing error occurs?
3 participants
No description provided.