test: unsubscribe #36
Conversation
matheusmosca
left a comment
matheusmosca
left a comment
There was a problem hiding this comment.
Hi @MelloTonio, sorry for taking too long to review your PR.
| return nil, ErrSubscriberNotFound | ||
| } | ||
|
|
||
| func (t Topic) UpdateTopic(topic Topic) (Topic, error) { |
There was a problem hiding this comment.
Could you remove this method?
| subsInterface := value.(Subscriber) | ||
|
|
||
| return &subsInterface, nil |
There was a problem hiding this comment.
| subsInterface := value.(Subscriber) | |
| return &subsInterface, nil | |
| subscriber := value.(Subscriber) | |
| return subscriber, nil |
| func (t *Topic) listenForSubscriptions() { | ||
| for newSubCh := range t.newSubCh { | ||
| t.subscribers.Store(newSubCh.GetID(), newSubCh) | ||
| func (t Topic) GetSubscriber(subscriberID vos.SubscriberID) (*Subscriber, error) { |
There was a problem hiding this comment.
Could you change this method to not return a pointer?
|
|
||
| //u.storage.UpdateTopic(ctx, topic) |
There was a problem hiding this comment.
Could you remove these lines?
| }, | ||
| beforeRun: func(topic entities.Topic) (chan vos.Message, vos.SubscriberID) { | ||
| subscriber := entities.NewSubscriber(topic) | ||
| ch, ID := subscriber.Subscribe() |
There was a problem hiding this comment.
could you write id in lowercase? I think variables should not start with capitalized letters.
| ch, ID := subscriber.Subscribe() | |
| ch, id := subscriber.Subscribe() |
| "github.com/matheusmosca/walrus/domain/entities" | ||
| ) | ||
|
|
||
| func (r *repository) UpdateTopic(ctx context.Context, topicName entities.Topic) (entities.Topic, error) { |
There was a problem hiding this comment.
Could you remove this file?
| subsCh, subsID := tt.beforeRun(topic) | ||
| defer close(subsCh) | ||
|
|
||
| tt.fields.storage = &RepositoryMock{ |
There was a problem hiding this comment.
I know there are some tests doing this, but could you define the RepositoryMock on each test case (I mean in the slice). You can turn fields into a function that receives a t *testing.T and an entities.Topic to prepare the repository.
| if tt.wantErr != nil { | ||
| assert.ErrorIs(t, err, tt.wantErr) | ||
| return | ||
| } |
There was a problem hiding this comment.
You are not asserting this error on successful scenarios.
| if tt.wantErr != nil { | |
| assert.ErrorIs(t, err, tt.wantErr) | |
| return | |
| } | |
| assert.ErrorIs(t, err, tt.wantErr) | |
| if tt.wantErr != nil { | |
| return | |
| } |
| fields fields | ||
| beforeRun func(topic entities.Topic) (chan vos.Message, vos.SubscriberID) | ||
| wantErr error | ||
| wantSubscriber bool |
There was a problem hiding this comment.
I think this field is not being used, could you remove it?
|
|
||
| type args struct { | ||
| ctx context.Context | ||
| message vos.Message |
There was a problem hiding this comment.
The unsubscribe use case does not have any relation with messages, could you remove this field? It makes more sense to add the topic name here (if you want to)
2 participants
TODO: