Service provider. Removes global services and passing services around. #61
Conversation
services/service_provider.go
Outdated
| err := storeService.Init() | ||
| if err != nil { | ||
| gaia.Cfg.Logger.Error("cannot initialize store", "error", err.Error()) | ||
| os.Exit(1) |
There was a problem hiding this comment.
Ups, this shouldn't be os.Exit(1) here but rather return an error.
|
I like that a lot. 😄 Currently, it feels a bit weird to pass around several instances of services. It also allows us to remove the public instance of store in handlers which is great. Some notes from my side:
Anyway, good job so far! ❤️ |
|
awesome! this makes it easier to write test. |
Codecov Report
@@ Coverage Diff @@
## master #61 +/- ##
==========================================
+ Coverage 57.37% 60.03% +2.66%
==========================================
Files 17 18 +1
Lines 1492 1544 +52
==========================================
+ Hits 856 927 +71
+ Misses 539 510 -29
- Partials 97 107 +10
Continue to review full report at Codecov.
|
|
@azbshiri @michelvocks I think you guys going to like this. ;) Take a look here at the mocked database test. https://github.com/gaia-pipeline/gaia/pull/61/files#diff-984f2526ba5ed92156cfe03873689506R285 With this we'll be able to completely remove all database dependency from all tests and use a mock. Expect for the store tests which can be left alone as integration tests. :) |
store/store.go
Outdated
|
|
||
| // Provides an embeddable interface so that not all of the methods need | ||
| // to be overriden in case of a Mock. | ||
| var _ GaiaStore = (*BoltStore)(nil) |
There was a problem hiding this comment.
This is pure magic right here. Allows for an embeddable type so that you don't have to implement every function in order to use a mock. Just the ones you would like to mock out like I'm demonstrating in the build_pipeline test.
There was a problem hiding this comment.
Actually no hahahahaha. :D I thought about this some more and it made no sense that it would influence embedability. This is just compile time interface compliance check for BoltStore. If boltstore would miss a function this line here wouldn't compile. Still neat that partial interface embedding works out of the box.
Skarlso
changed the title
|
Scheduler is now also behind an interface. This made me able to write three additional tests for handlers which further increased coverage. |
|
Lovely! 🤗 |
|
Waiting for #60 to go in. After that, refactor and it should be ready to be merged. Also, once Java support is merged, I'll increase the test coverage with the help of the service provider. |
|
Alright, stuff is merged. Now doing the cleanup... :P |
Skarlso
changed the title
|
Doneeeeeeeeee 🎉 @michelvocks So this should be now ready to be merged. I increased the coverage too. There are some fringe cases that could still be covered though. But I want to unblock other PRs first, and then add more tests there. What say you? Also, if you are wondering why the error is ignored from the services, that's because of the singleton. All the services are initialised in the main function. If they don't work, the main function exits with os.Exit(1). This means the rest of the application is safe, since from there on the singleton is returned always. |
|
Well done, @Skarlso 🤗 |
No description provided.