Added tests + automatic test/coverage in travis + docs/readme #16
Conversation
Signed-off-by: cgparini <claudio.gasparini@pantheon.tech>
# Conflicts: # Makefile # glide.lock
bgp/bgp_api_test.go
Outdated
|
|
||
| import ( | ||
| "github.com/ligato/cn-infra/logging/logrus" | ||
| "github.com/stretchr/testify/assert" |
There was a problem hiding this comment.
please use gomega to unify assertions all over the ligato
bgp/bgp_api_test.go
Outdated
| "testing" | ||
| ) | ||
|
|
||
| func TestToChan(t *testing.T) { |
There was a problem hiding this comment.
Please explain in comment what is the intent of this test
bgp/gobgp/plugin_impl_gobgp_test.go
Outdated
| goBGPPlugin := createGoBGPPlugin(serverConf) | ||
|
|
||
| // watch -> start lifecycle of gobgp plugin -> send path to Route reflector -> check receiving on other end | ||
| watchRegistration, registrationErr := goBGPPlugin.WatchIPRoutes("TestWatcher", bgp.ToChan(channel, logroot.StandardLogger())) |
There was a problem hiding this comment.
No need to use ToChan in here is it?
There was a problem hiding this comment.
Basically no need to use callback either.
I chose channel because:
- I had some methods (outdated test from Claudio) that used channel
- for callback i had to do some struct that would record what came using callback. Channel is build in, so one struct less.
bgp/gobgp/plugin_impl_gobgp_test.go
Outdated
|
|
||
| var flavor = &local.FlavorLocal{} | ||
|
|
||
| func TestGoBGPPluginInfoPassing(t *testing.T) { |
There was a problem hiding this comment.
Please explain in comment what is the intent of this test.
Consider having: Given/When/Then test steps to do the test scenario readable.
Take a look at https://github.com/ligato/sfc-controller/pull/1/files#diff-b2ae6240742894fb8ffcd1d9406f9ad3R33 it it helps you....
Remove duplicated package under glide.yaml
…ere(probably some type resolving of gotypex)
|
Changes Unknown when pulling 5d9de6e on fgschwan:master into ** on ligato:master**. |
|
Changes Unknown when pulling daed210 on fgschwan:master into ** on ligato:master**. |
fgschwan
changed the title
|
Changes Unknown when pulling 907e75b on fgschwan:master into ** on ligato:master**. |
bgp/gobgp/plugin_impl_gobgp_test.go
Outdated
| timeoutForNotReceiving = 5 * time.Second | ||
| ) | ||
|
|
||
| var flavor = &local.FlavorLocal{} |
There was a problem hiding this comment.
let's not use global variables other than test data or test configuration
bgp/gobgp/plugin_impl_gobgp_test.go
Outdated
| Expect(watchRegistration).NotTo(BeNil(), "WatchRegistration must be non-nil to be able to close registration later") | ||
| lifecycleCloseChannel := startPluginLifecycle(goBGPPlugin, assertCorrectLifecycleEnd(&lifecycleWG)) | ||
| Expect(waitForSessionEstablishment(routeReflector)).To(BeNil(), "Session not established within timeout") | ||
| Expect(addNewRoute(routeReflector, prefix1, nextHop1, length)).To(BeNil(), "Can't add new route") |
There was a problem hiding this comment.
When.addNewRoute()
Then.assertThatChannelReceivesCorrectRoute
bgp/gobgp/plugin_impl_gobgp_test.go
Outdated
| var lifecycleWG sync.WaitGroup | ||
|
|
||
| // create components | ||
| routeReflector, err := createAndStartRouteReflector(routeReflectorConf) |
There was a problem hiding this comment.
Given.RouteReflectorGoBGP
(extract all intermediate assertions away from test scenario to make it readable)
bgp/gobgp/plugin_impl_gobgp_test.go
Outdated
| watchRegistration, registrationErr := goBGPPlugin.WatchIPRoutes("TestWatcher", bgp.ToChan(channel, logroot.StandardLogger())) | ||
| Expect(registrationErr).To(BeNil(), "Can't properly register to watch IP routes") | ||
| Expect(watchRegistration).NotTo(BeNil(), "WatchRegistration must be non-nil to be able to close registration later") | ||
| lifecycleCloseChannel := startPluginLifecycle(goBGPPlugin, assertCorrectLifecycleEnd(&lifecycleWG)) |
There was a problem hiding this comment.
Given.AgentPluginGoBGP
(extract all intermediate assertions away from test scenario to make it readable)
bgp/gobgp/plugin_impl_gobgp_test.go
Outdated
|
|
||
| // TestGoBGPPluginInfoPassing tests gobgp plugin for the ability of retrieving of ReachableIPRoutes using BGP protocol and passing this information to its registered watchers. | ||
| // Test is also testing the ability of watch unregistering. | ||
| func TestGoBGPPluginInfoPassing(t *testing.T) { |
There was a problem hiding this comment.
In general:
- emphasize that you are starting dedicated GoBGP Route Reflector (because once somebody wants to add yet another test there will be concurrency problem - two tests would want to start GoBGP Route Reflector)
- Extract intermediate assertions of return values away from scenario (starting new thing - checking not error, watch registration checking not error)
- Move test data, test configuration, test helper functions to dedicated file
|
Changes Unknown when pulling 1701a77 on fgschwan:master into ** on ligato:master**. |
No description provided.