Turned NewDBConnection into a singleton, added database tests.

[andrew-sledge]

No description provided.

[duaraghav8]

Do we really need both err and dbErr variables?
Since we will anyway return db, err, why don't we get rid of dbErr var?
In case of error, we directly assign our error to err, like:

err = fmt.Errorf(...)

Then, just returning db, err is good enough.

Am I missing something here?

[andrew-sledge]

The thought was a little in preparation for structured logging. For instance if you want to wrap the error with an error code, datetime, etc. the error returned from gorm may not completely wrap it in the way that you want. I'm not too passionate about it so if you want it changed I can do that - just trying to save a couple of steps down the road.

[duaraghav8]

This test currently fails always.
This is because the NewDBConnection() already ran once for the first test so it returns a db and no error, which fails this test.

If you re-order the tests, then the other one will fail. Only the first test passes because the second time, sync.Once() doesn't run the logic again (which is expected behavior).

Is there a way to "reset" the NewDBConnection() function? We must reset it for every testcase.

[andrew-sledge]

Good catch - I was running these individually in my IDE. Working on this...

[andrew-sledge]

One thing to think about - instead of using a singleton it might be preferable to use dependency injection. DI would make testing easier and give you more flexibility for data connection types because it would implement an interface.

[duaraghav8]

@andrew-sledge I'm open to dependency injection too. The singleton idea came up because the DB object is currently its own package.
How do you think this can be done?
Either we create the DB object in the start command and inject it into the server

[andrew-sledge]

@duaraghav8 yep - that's probably the best way.