Refactor/guble 24 public router interface #46
Conversation
Fixed issue where messages would've been stored although no WRITE access was allowed.
| routerMock.EXPECT().KVStore().Return(store.NewMemoryKVStore(), nil) | ||
| routerMock.EXPECT().AccessManager().Return(amMock, nil).MaxTimes(2) | ||
| routerMock.EXPECT().MessageStore().Return(msMock, nil).MaxTimes(2) |
There was a problem hiding this comment.
Maybe it would be better to have a had written struct based implementation of the router interface for testing,
then I could write something like
routerMock := routerMock{MessageStore: NewMockMessageStore(mockCtrl), ...}
There was a problem hiding this comment.
This is good point. Want this now in this PR?
There was a problem hiding this comment.
I prefer to have all things done before merging,
but if you prefer, I also can merge directly.
There was a problem hiding this comment.
I will add this first.
I have an idea here to extend the mock from the current mock generated in which the call to these methods return the supplied objects.
There was a problem hiding this comment.
There is a problem with this. Here I'm testing something in gubled using a mock from server (RouterMock). Re your comment above this means I should either:
- create a
RouterMockinside thegubledpackage in a_test.gofile and it will be visible only here. - create a public
RouterMockin theserverpackage and use it in other packages.
The other solution is to leave it as is, and everyone would supply the arguments according to the test.
There was a problem hiding this comment.
Would be the same with the generated mock.
I would never leave the mock code in the production code, so I see two possible solutions:
- have the mock redundant in the _test.go code of each package, where we need it
- Create a separate test_util package
In this small case I would prefer the redundancy variant.
|
It was a good decision to make this change: Make the code much simpler. |
|
I would merge it now, but the build fails. |
|
The build fails, but for us works ok locally. We will focus on test fixing from now in next PR's. |
No description provided.