Add client list, fix channel race, hjoin, tests #40
Conversation
| @@ -71,3 +83,68 @@ func TestClient(t *testing.T) { | |||
|
|
|||
| } | |||
| } | |||
|
|
|||
| // TestMultipleClients | |||
There was a problem hiding this comment.
These test cases tend to deadlock and need to be redesigned.
Edit: Fixed the test. The pipe acts like an unbuffered channel and blocks until the data is read. I modified the test handling to read from all the clients at once in different goroutines and then handle them sequentially once they had been collated. I also modified it to fully read everything before moving on to make it easier to debug/create tests.
There was a problem hiding this comment.
Even if the code is valuable, I think the your code to solve this issue is too complex. I need some time to think about it.
Could you do a PR for the testing only? This is something I'm happy to apply.
There was a problem hiding this comment.
Where did we leave off on this? @kdedon did you intend to break this into 2 pieces? I hate to have this dangling out here for so long.
| } | ||
|
|
||
| func (c *Client) String() string { | ||
| return c.Name | ||
| } | ||
|
|
||
| func newClient(conn *net.TCPConn) *Client { | ||
| func newClient(conn net.Conn) *Client { |
There was a problem hiding this comment.
This change makes it much easier to build tests since the entire server doesn't have to be constructed.
Fixed a bug with hjoin, username tests to include the @, and removed the test for a number as the first character due to that being an unnecessary check after testing for the @ above.
I handled the two TODOs:
Add after the remove is handled by just writing the channel back into the global index. There is still a race condition where a user could /join during the time that the channel was deleted but while the add was still queued, so it is not completely fixed; I think it might be safest to just use a regular map and use an RWLock Mutex for any possible operation.
I added a client channel slice. I liberally used appends, so not sure how much of an optimization it actually was.