Writing Test coverage for ReadMigrations function #3397
Conversation
Signed-off-by: shivam <[email protected]>
|
@richscott I am working on creating Test coverage for the function
There are many changes required like we can add/remove conditions which will make things more better .... |
| ) | ||
|
|
||
| func TestReadMigrations(t *testing.T) { | ||
| mockFiles := []string{ |
There was a problem hiding this comment.
This is a good start - using a mocked filesystem with io/fs is good. Try adding some more files in here, with varying differences to match (or not) the expected patterns) - see the existing migrations in internal/scheduler/database/migrations/*.sql and also in internal/lookoutv2/schema/migrations/*.sql.
There was a problem hiding this comment.
Sure, will look into it
| } | ||
| assert.Len(t, migrations, 1, "Unexpected slice length") | ||
|
|
||
| // Slice first value should be int |
There was a problem hiding this comment.
The comments for these assertions are nice, but probably not necessary - the code is very clear.
| } | ||
|
|
||
| /* | ||
| func createMockFileSystem(files []string) fs.FS { |
There was a problem hiding this comment.
Here you just need to declare a new struct type, and then have it implement the same method(s) that are defined for fs.FS - see https://pkg.go.dev/io/[email protected]
There was a problem hiding this comment.
Can you please elaborate this one?
There was a problem hiding this comment.
This would be just an instance of creating a classic Go interface implementation. You'd create a struct (for example, type struct testFS { ... }) and then create method(s) that implement the fs.FS interface - in this case, there's just one method, so the new function definition would be
func (myFS *testFS) Open(name string) (File, error) { /* return a fake File object */ }.
If you search for golang interfaces, there are several good articles on the basics of implementing interfaces in Go, like https://golangbot.com/interfaces-part-1/ .
There was a problem hiding this comment.
I have created struct testFS for Mockfiles handling
Signed-off-by: shivam <[email protected]>
|
Hello @richscott I have modified tests. I aligned on to match the Currently the Tests includes-
btw.... it needs modifcation as currently |
|
can you please @richscott review the current tests which i have pushed |
Signed-off-by: shivam <[email protected]>
| @@ -98,7 +98,7 @@ func ReadMigrations(fsys fs.FS, basePath string) ([]Migration, error) { | |||
|
|
|||
| var migrations []Migration | |||
| for _, f := range files { | |||
|
|
|||
There was a problem hiding this comment.
This change should be removed.
|
|
||
| assert.Equal(t, migrations[0].id, 1, "Incorrect ID for 001_initial_schema.sql") | ||
| assert.Equal(t, migrations[1].id, 2, "Incorrect ID for 002_cancel_reason.sql") | ||
| assert.Equal(t, migrations[2].id, 3, "Incorrect ID for 003_run_leased_column.sql") |
There was a problem hiding this comment.
I don't think it's necessary to exhaustively test for ID matching for every existing SQL file - just two or three sample tests should be enough.
Signed-off-by: shivam <[email protected]>
|
Hey @richscott I have made the changes.. |
| package database | ||
|
|
||
| import ( | ||
| "testing" |
There was a problem hiding this comment.
Can you run the goimports tool on this file? It will sort the imports, and separate out third-party/non-standard-library modules, automatically.
There was a problem hiding this comment.
Hey @richscott When i run goimports -w migrations_test.go It says command not found but I have installed it.
I am running from the path /home/shivam/armada/internal/common/database
What type of PR is this?
I am creating Test Coverage for the function ReadMigrations
Which issue(s) this PR fixes:
refs: #3390