Epic final commit for nick/chan/logging/testing refactor.

* Brings logging changes to client library.
* Brings state tracker to client library.
* Rewrites all tests to use mock logger and mock state tracker.
* Makes state tracking optional, finally.
* Shaves yaks until they are almost completely bald.

client/handlers_test.go

@@ -1,6 +1,8 @@
 package client
 
 import (
+	"github.com/fluffle/goirc/state"
+	"gomock.googlecode.com/hg/gomock"
 	"testing"
 )
 
@@ -9,28 +11,25 @@ import (
 // in this file will call their respective handlers synchronously, otherwise
 // testing becomes more difficult.
 func TestPING(t *testing.T) {
-	m, c := setUp(t)
-	defer tearDown(m, c)
-	m.Send("PING :1234567890")
-	m.Expect("PONG :1234567890")
+	_, s := setUp(t)
+	defer s.tearDown()
+	s.nc.Send("PING :1234567890")
+	s.nc.Expect("PONG :1234567890")
 }
 
 // Test the handler for 001 / RPL_WELCOME
 func Test001(t *testing.T) {
-	m, c := setUp(t)
-	defer tearDown(m, c)
+	c, s := setUp(t)
+	defer s.tearDown()
 
 	// Setup a mock event dispatcher to test correct triggering of "connected"
-	flag := false
-	c.Dispatcher = WasEventDispatched("connected", &flag)
+	flag := c.ExpectEvent("connected")
 
 	// Call handler with a valid 001 line
 	c.h_001(parseLine(":irc.server.org 001 test :Welcome to IRC test!ident@somehost.com"))
-	// Should result in no response to server
-	m.ExpectNothing()
 
 	// Check that the event was dispatched correctly
-	if !flag {
+	if !*flag {
 		t.Errorf("Sending 001 didn't result in dispatch of connected event.")
 	}
 
@@ -42,12 +41,12 @@ func Test001(t *testing.T) {
 
 // Test the handler for 433 / ERR_NICKNAMEINUSE
 func Test433(t *testing.T) {
-	m, c := setUp(t)
-	defer tearDown(m, c)
+	c, s := setUp(t)
+	defer s.tearDown()
 
 	// Call handler with a 433 line, not triggering c.Me.Renick()
 	c.h_433(parseLine(":irc.server.org 433 test new :Nickname is already in use."))
-	m.Expect("NICK new_")
+	s.nc.Expect("NICK new_")
 
 	// In this case, we're expecting the server to send a NICK line
 	if c.Me.Nick != "test" {
@@ -58,544 +57,479 @@ func Test433(t *testing.T) {
 	// nick is unavailable during initial negotiation, so we must choose a
 	// different one before the connection can proceed. No NICK line will be
 	// sent by the server to confirm nick change in this case.
+	s.st.EXPECT().ReNick("test", "test_")
 	c.h_433(parseLine(":irc.server.org 433 test test :Nickname is already in use."))
-	m.Expect("NICK test_")
+	s.nc.Expect("NICK test_")
+
+	// Counter-intuitively, c.Me.Nick will not change in this case. This is an
+	// artifact of the test set-up, with a mocked out state tracker that
+	// doesn't actually change any state. Normally, this would be fine :-)
+	if c.Me.Nick != "test" {
+		t.Errorf("My nick changed from '%s'.", c.Me.Nick)
+	}
+
+	// Test the code path that *doesn't* involve state tracking.
+	c.st = false
+	c.h_433(parseLine(":irc.server.org 433 test test :Nickname is already in use."))
+	s.nc.Expect("NICK test_")
 
 	if c.Me.Nick != "test_" {
-		t.Errorf("ReNick() not called, Nick == '%s'.", c.Me.Nick)
+		t.Errorf("My nick not updated from '%s'.", c.Me.Nick)
 	}
 }
 
-// Test the handler for NICK messages
+// Test the handler for NICK messages when state tracking is disabled
 func TestNICK(t *testing.T) {
-	m, c := setUp(t)
-	defer tearDown(m, c)
+	c, s := setUp(t)
+	defer s.tearDown()
+
+	// State tracking is enabled by default in setUp
+	c.st = false
 
 	// Call handler with a NICK line changing "our" nick to test1.
 	c.h_NICK(parseLine(":test!test@somehost.com NICK :test1"))
-	// Should generate no response to server
-	m.ExpectNothing()
 
 	// Verify that our Nick has changed
 	if c.Me.Nick != "test1" {
 		t.Errorf("NICK did not result in changing our nick.")
 	}
 
-	// Create a "known" nick other than ours
-	user1 := c.NewNick("user1", "ident1", "name one", "host1.com")
-
-	// Call handler with a NICK line changing user1 to somebody
-	c.h_NICK(parseLine(":user1!ident1@host1.com NICK :somebody"))
-	m.ExpectNothing()
-
-	if c.GetNick("user1") != nil {
-		t.Errorf("Still have a valid Nick for 'user1'.")
-	}
-	if n := c.GetNick("somebody"); n != user1 {
-		t.Errorf("GetNick(somebody) didn't result in correct Nick.")
-	}
-
-	// Send a NICK line for an unknown nick.
+	// Send a NICK line for something that isn't us.
 	c.h_NICK(parseLine(":blah!moo@cows.com NICK :milk"))
-	m.ExpectNothing()
+
+	// Verify that our Nick hasn't changed
+	if c.Me.Nick != "test1" {
+		t.Errorf("NICK did not result in changing our nick.")
+	}
+
+	// Re-enable state tracking and send a line that *should* change nick.
+	c.st = true
+	c.h_NICK(parseLine(":test1!test@somehost.com NICK :test2"))
+
+	// Verify that our Nick hasn't changed (should be handled by h_STNICK).
+	if c.Me.Nick != "test1" {
+		t.Errorf("NICK changed our nick when state tracking enabled.")
+	}
 }
 
 // Test the handler for CTCP messages
 func TestCTCP(t *testing.T) {
-	m, c := setUp(t)
-	defer tearDown(m, c)
+	c, s := setUp(t)
+	defer s.tearDown()
 
 	// Call handler with CTCP VERSION
 	c.h_CTCP(parseLine(":blah!moo@cows.com PRIVMSG test :\001VERSION\001"))
 
 	// Expect a version reply
-	m.Expect("NOTICE blah :\001VERSION powered by goirc...\001")
+	s.nc.Expect("NOTICE blah :\001VERSION powered by goirc...\001")
 
 	// Call handler with CTCP PING
 	c.h_CTCP(parseLine(":blah!moo@cows.com PRIVMSG test :\001PING 1234567890\001"))
 
 	// Expect a ping reply
-	m.Expect("NOTICE blah :\001PING 1234567890\001")
+	s.nc.Expect("NOTICE blah :\001PING 1234567890\001")
 
 	// Call handler with CTCP UNKNOWN
 	c.h_CTCP(parseLine(":blah!moo@cows.com PRIVMSG test :\001UNKNOWN ctcp\001"))
-
-	// Expect nothing in reply
-	m.ExpectNothing()
 }
 
 // Test the handler for JOIN messages
 func TestJOIN(t *testing.T) {
-	// TODO(fluffle): Without mocking to ensure that the various methods
-	// h_JOIN uses are called, we must check they do the right thing by
-	// verifying their expected side-effects instead. Fixing this requires
-	// significant effort to move Conn to being a mockable interface type
-	// instead of a concrete struct. I'm not sure how feasible this is :-/
-	//
-	// Soon, we'll find out :-)
+	c, s := setUp(t)
+	defer s.tearDown()
 
-	m, c := setUp(t)
-	defer tearDown(m, c)
+	// The state tracker should be creating a new channel in this first test
+	chan1 := state.NewChannel("#test1", s.log)
+
+	gomock.InOrder(
+		s.st.EXPECT().GetChannel("#test1").Return(nil),
+		s.st.EXPECT().GetNick("test").Return(c.Me),
+		s.st.EXPECT().NewChannel("#test1").Return(chan1),
+		s.st.EXPECT().Associate(chan1, c.Me),
+	)
 
 	// Use #test1 to test expected behaviour
 	// Call handler with JOIN by test to #test1
 	c.h_JOIN(parseLine(":test!test@somehost.com JOIN :#test1"))
 
 	// Verify that the MODE and WHO commands are sent correctly
-	m.Expect("MODE #test1")
-	m.Expect("WHO #test1")
+	s.nc.Expect("MODE #test1")
+	s.nc.Expect("WHO #test1")
 
-	// Simple verification that NewChannel was called for #test1
-	test1 := c.GetChannel("#test1")
-	if test1 == nil {
-		t.Errorf("No Channel for #test1 created on JOIN.")
-	}
+	// In this second test, we should be creating a new nick
+	nick1 := state.NewNick("user1", s.log)
+
+	gomock.InOrder(
+		s.st.EXPECT().GetChannel("#test1").Return(chan1),
+		s.st.EXPECT().GetNick("user1").Return(nil),
+		s.st.EXPECT().NewNick("user1").Return(nick1),
+		s.st.EXPECT().Associate(chan1, nick1),
+	)
 
 	// OK, now #test1 exists, JOIN another user we don't know about
 	c.h_JOIN(parseLine(":user1!ident1@host1.com JOIN :#test1"))
 
 	// Verify that the WHO command is sent correctly
-	m.Expect("WHO user1")
+	s.nc.Expect("WHO user1")
 
-	// Simple verification that NewNick was called for user1
-	user1 := c.GetNick("user1")
-	if user1 == nil {
-		t.Errorf("No Nick for user1 created on JOIN.")
-	}
-
-	// Now, JOIN a nick we *do* know about.
-	user2 := c.NewNick("user2", "ident2", "name two", "host2.com")
+	// In this third test, we'll be pretending we know about the nick already.
+	nick2 := state.NewNick("user2", c.l)
+	gomock.InOrder(
+		s.st.EXPECT().GetChannel("#test1").Return(chan1),
+		s.st.EXPECT().GetNick("user2").Return(nick2),
+		s.st.EXPECT().Associate(chan1, nick2),
+	)
 	c.h_JOIN(parseLine(":user2!ident2@host2.com JOIN :#test1"))
 
-	// We already know about this user and channel, so nothing should be sent
-	m.ExpectNothing()
-
-	// Simple verification that the state tracking has actually been done
-	if _, ok := test1.Nicks[user2]; !ok || len(test1.Nicks) != 3 {
-		t.Errorf("State tracking horked, hopefully other unit tests fail.")
-	}
-
-	// Test error paths -- unknown channel, unknown nick
+	// Test error paths
+	gomock.InOrder(
+		// unknown channel, unknown nick
+		s.st.EXPECT().GetChannel("#test2").Return(nil),
+		s.st.EXPECT().GetNick("blah").Return(nil),
+		s.log.EXPECT().Warn("irc.JOIN(): JOIN to unknown channel %s "+
+			"received from (non-me) nick %s", "#test2", "blah"),
+		// unknown channel, known nick that isn't Me.
+		s.st.EXPECT().GetChannel("#test2").Return(nil),
+		s.st.EXPECT().GetNick("user2").Return(nick2),
+		s.log.EXPECT().Warn("irc.JOIN(): JOIN to unknown channel %s "+
+			"received from (non-me) nick %s", "#test2", "user2"),
+	)
 	c.h_JOIN(parseLine(":blah!moo@cows.com JOIN :#test2"))
-	m.ExpectNothing()
-
-	// unknown channel, known nick that isn't Me.
 	c.h_JOIN(parseLine(":user2!ident2@host2.com JOIN :#test2"))
-	m.ExpectNothing()
 }
 
 // Test the handler for PART messages
 func TestPART(t *testing.T) {
-	m, c := setUp(t)
-	defer tearDown(m, c)
+	c, s := setUp(t)
+	defer s.tearDown()
 
-	// Create user1 and add them to #test1 and #test2
-	user1 := c.NewNick("user1", "ident1", "name one", "host1.com")
-	test1 := c.NewChannel("#test1")
-	test2 := c.NewChannel("#test2")
-	test1.AddNick(user1)
-	test2.AddNick(user1)
+	// We need some valid and associated nicks / channels to PART with.
+	chan1 := state.NewChannel("#test1", s.log)
+	nick1 := state.NewNick("user1", s.log)
 
-	// Add Me to both channels (not strictly necessary)
-	test1.AddNick(c.Me)
-	test2.AddNick(c.Me)
-
-	// Then make them PART
+	// PART should dissociate a nick from a channel.
+	gomock.InOrder(
+		s.st.EXPECT().GetChannel("#test1").Return(chan1),
+		s.st.EXPECT().GetNick("user1").Return(nick1),
+		s.st.EXPECT().Dissociate(chan1, nick1),
+	)
 	c.h_PART(parseLine(":user1!ident1@host1.com PART #test1 :Bye!"))
-
-	// Expect no output
-	m.ExpectNothing()
-
-	// Quick check of tracking code
-	if len(test1.Nicks) != 1 {
-		t.Errorf("PART failed to remove user1 from #test1.")
-	}
-
-	// Test error states.
-	// Part a known user from a known channel they are not on.
-	c.h_PART(parseLine(":user1!ident1@host1.com PART #test1 :Bye!"))
-
-	// Part an unknown user from a known channel.
-	c.h_PART(parseLine(":user2!ident2@host2.com PART #test1 :Bye!"))
-
-	// Part a known user from an unknown channel.
-	c.h_PART(parseLine(":user1!ident1@host1.com PART #test3 :Bye!"))
-
-	// Part an unknown user from an unknown channel.
-	c.h_PART(parseLine(":user2!ident2@host2.com PART #test3 :Bye!"))
 }
 
 // Test the handler for KICK messages
 // (this is very similar to the PART message test)
 func TestKICK(t *testing.T) {
-	m, c := setUp(t)
-	defer tearDown(m, c)
+	c, s := setUp(t)
+	defer s.tearDown()
 
-	// Create user1 and add them to #test1 and #test2
-	user1 := c.NewNick("user1", "ident1", "name one", "host1.com")
-	test1 := c.NewChannel("#test1")
-	test2 := c.NewChannel("#test2")
-	test1.AddNick(user1)
-	test2.AddNick(user1)
+	// We need some valid and associated nicks / channels to KICK.
+	chan1 := state.NewChannel("#test1", s.log)
+	nick1 := state.NewNick("user1", s.log)
 
-	// Add Me to both channels (not strictly necessary)
-	test1.AddNick(c.Me)
-	test2.AddNick(c.Me)
-
-	// Then kick them!
+	// KICK should dissociate a nick from a channel.
+	gomock.InOrder(
+		s.st.EXPECT().GetChannel("#test1").Return(chan1),
+		s.st.EXPECT().GetNick("user1").Return(nick1),
+		s.st.EXPECT().Dissociate(chan1, nick1),
+	)
 	c.h_KICK(parseLine(":test!test@somehost.com KICK #test1 user1 :Bye!"))
-
-	// Expect no output
-	m.ExpectNothing()
-
-	// Quick check of tracking code
-	if len(test1.Nicks) != 1 {
-		t.Errorf("PART failed to remove user1 from #test1.")
-	}
-
-	// Test error states.
-	// Kick a known user from a known channel they are not on.
-	c.h_KICK(parseLine(":test!test@somehost.com KICK #test1 user1 :Bye!"))
-
-	// Kick an unknown user from a known channel.
-	c.h_KICK(parseLine(":test!test@somehost.com KICK #test2 user2 :Bye!"))
-
-	// Kick a known user from an unknown channel.
-	c.h_KICK(parseLine(":test!test@somehost.com KICK #test3 user1 :Bye!"))
-
-	// Kick an unknown user from an unknown channel.
-	c.h_KICK(parseLine(":test!test@somehost.com KICK #test4 user2 :Bye!"))
 }
 
 // Test the handler for QUIT messages
 func TestQUIT(t *testing.T) {
-	m, c := setUp(t)
-	defer tearDown(m, c)
+	c, s := setUp(t)
+	defer s.tearDown()
 
-	// Create user1 and add them to #test1 and #test2
-	user1 := c.NewNick("user1", "ident1", "name one", "host1.com")
-	test1 := c.NewChannel("#test1")
-	test2 := c.NewChannel("#test2")
-	test1.AddNick(user1)
-	test2.AddNick(user1)
-
-	// Add Me to both channels (not strictly necessary)
-	test1.AddNick(c.Me)
-	test2.AddNick(c.Me)
-
-	// Have user1 QUIT
+	// Have user1 QUIT. All possible errors handled by state tracker \o/
+	s.st.EXPECT().DelNick("user1")
 	c.h_QUIT(parseLine(":user1!ident1@host1.com QUIT :Bye!"))
-
-	// Expect no output
-	m.ExpectNothing()
-
-	// Quick check of tracking code
-	if len(test1.Nicks) != 1 || len(test2.Nicks) != 1 {
-		t.Errorf("QUIT failed to remove user1 from channels.")
-	}
-
-	// Ensure user1 is no longer a known nick
-	if c.GetNick("user1") != nil {
-		t.Errorf("QUIT failed to remove user1 from state tracking completely.")
-	}
-
-	// Have user1 QUIT again, expect ERRORS!
-	c.h_QUIT(parseLine(":user1!ident1@host1.com QUIT :Bye!"))
-
-	// Have a previously unmentioned user quit, expect an error
-	c.h_QUIT(parseLine(":user2!ident2@host2.com QUIT :Bye!"))
 }
 
 // Test the handler for MODE messages
 func TestMODE(t *testing.T) {
-	m, c := setUp(t)
-	defer tearDown(m, c)
+	c, s := setUp(t)
+	defer s.tearDown()
 
-	// Create user1 and add them to #test1
-	user1 := c.NewNick("user1", "ident1", "name one", "host1.com")
-	test1 := c.NewChannel("#test1")
-	test1.AddNick(user1)
-	test1.AddNick(c.Me)
-	cm := test1.Modes
+	chan1 := state.NewChannel("#test1", s.log)
+	nick1 := state.NewNick("user1", s.log)
 
-	// Verify the ChanPrivs exists and modes we're testing aren't set
-	if cp, ok := user1.Channels[test1]; !ok || c.Me.Channels[test1].Voice ||
-		cp.Op || cm.Key != "" || cm.InviteOnly || cm.Secret {
-		t.Errorf("Channel privileges in unexpected state before MODE.")
+	// Send a channel mode line. Inconveniently, Channel and Nick objects
+	// aren't mockable with gomock as they're not interface types (and I
+	// don't want them to be, writing accessors for struct fields sucks).
+	// This makes testing whether ParseModes is called correctly harder.
+	s.st.EXPECT().GetChannel("#test1").Return(chan1)
+	c.h_MODE(parseLine(":user1!ident1@host1.com MODE #test1 +sk somekey"))
+	if !chan1.Modes.Secret || chan1.Modes.Key != "somekey" {
+		t.Errorf("Channel.ParseModes() not called correctly.")
 	}
 
-	// Send a channel mode line
-	c.h_MODE(parseLine(":user1!ident1@host1.com MODE #test1 +kisvo somekey test user1"))
 
-	// Expect no output
-	m.ExpectNothing()
-
-	// Verify expected state afterwards.
-	if cp := user1.Channels[test1]; !(cp.Op || c.Me.Channels[test1].Voice ||
-		cm.Key != "somekey" || cm.InviteOnly || cm.Secret) {
-		t.Errorf("Channel privileges in unexpected state after MODE.")
+	// Send a nick mode line, returning Me
+	gomock.InOrder(
+		s.st.EXPECT().GetChannel("test").Return(nil),
+		s.st.EXPECT().GetNick("test").Return(c.Me),
+	)
+	c.h_MODE(parseLine(":test!test@somehost.com MODE test +i"))
+	if !c.Me.Modes.Invisible {
+		t.Errorf("Nick.ParseModes() not called correctly.")
 	}
 
-	// Verify our nick modes are what we expect before test
-	nm := c.Me.Modes
-	if nm.Invisible || nm.WallOps || nm.HiddenHost {
-		t.Errorf("Our nick privileges in unexpected state before MODE.")
-	}
-
-	// Send a nick mode line
-	c.h_MODE(parseLine(":test!test@somehost.com MODE test +ix"))
-	m.ExpectNothing()
-
-	// Verify the two modes we expect to change did so
-	if !nm.Invisible || nm.WallOps || !nm.HiddenHost {
-		t.Errorf("Our nick privileges in unexpected state after MODE.")
-	}
-
-	// Check error paths -- send a valid user mode that's not us
+	// Check error paths
+	gomock.InOrder(
+		// send a valid user mode that's not us
+		s.st.EXPECT().GetChannel("user1").Return(nil),
+		s.st.EXPECT().GetNick("user1").Return(nick1),
+		s.log.EXPECT().Warn("irc.MODE(): recieved MODE %s for (non-me) nick %s",
+			"+w", "user1"),
+		// Send a random mode for an unknown channel
+		s.st.EXPECT().GetChannel("#test2").Return(nil),
+		s.st.EXPECT().GetNick("#test2").Return(nil),
+		s.log.EXPECT().Warn("irc.MODE(): not sure what to do with MODE %s",
+			"#test2 +is"),
+	)
 	c.h_MODE(parseLine(":user1!ident1@host1.com MODE user1 +w"))
-	m.ExpectNothing()
-
-	// Send a random mode for an unknown channel
 	c.h_MODE(parseLine(":user1!ident1@host1.com MODE #test2 +is"))
-	m.ExpectNothing()
 }
 
 // Test the handler for TOPIC messages
 func TestTOPIC(t *testing.T) {
-	m, c := setUp(t)
-	defer tearDown(m, c)
+	c, s := setUp(t)
+	defer s.tearDown()
 
-	// Create #test1 so we have a channel to set the topic of
-	test1 := c.NewChannel("#test1")
+	chan1 := state.NewChannel("#test1", s.log)
 
 	// Assert that it has no topic originally
-	if test1.Topic != "" {
+	if chan1.Topic != "" {
 		t.Errorf("Test channel already has a topic.")
 	}
 
 	// Send a TOPIC line
+	s.st.EXPECT().GetChannel("#test1").Return(chan1)
 	c.h_TOPIC(parseLine(":user1!ident1@host1.com TOPIC #test1 :something something"))
-	m.ExpectNothing()
 
 	// Make sure the channel's topic has been changed
-	if test1.Topic != "something something" {
+	if chan1.Topic != "something something" {
 		t.Errorf("Topic of test channel not set correctly.")
 	}
 
 	// Check error paths -- send a topic for an unknown channel
+	gomock.InOrder(
+		s.st.EXPECT().GetChannel("#test2").Return(nil),
+		s.log.EXPECT().Warn("irc.TOPIC(): topic change on unknown channel %s",
+			"#test2"),
+	)
 	c.h_TOPIC(parseLine(":user1!ident1@host1.com TOPIC #test2 :dark side"))
-	m.ExpectNothing()
 }
 
 // Test the handler for 311 / RPL_WHOISUSER
 func Test311(t *testing.T) {
-	m, c := setUp(t)
-	defer tearDown(m, c)
+	c, s := setUp(t)
+	defer s.tearDown()
 
 	// Create user1, who we know little about
-	user1 := c.NewNick("user1", "", "", "")
+	nick1 := state.NewNick("user1", s.log)
 
 	// Send a 311 reply
+	s.st.EXPECT().GetNick("user1").Return(nick1)
 	c.h_311(parseLine(":irc.server.org 311 test user1 ident1 host1.com * :name"))
-	m.ExpectNothing()
 
 	// Verify we now know more about user1
-	if user1.Ident != "ident1" ||
-		user1.Host != "host1.com" ||
-		user1.Name != "name" {
+	if nick1.Ident != "ident1" ||
+		nick1.Host != "host1.com" ||
+		nick1.Name != "name" {
 		t.Errorf("WHOIS info of user1 not set correctly.")
 	}
 
 	// Check error paths -- send a 311 for an unknown nick
+	gomock.InOrder(
+		s.st.EXPECT().GetNick("user2").Return(nil),
+		s.log.EXPECT().Warn("irc.311(): received WHOIS info for unknown nick %s",
+			"user2"),
+	)
 	c.h_311(parseLine(":irc.server.org 311 test user2 ident2 host2.com * :dongs"))
-	m.ExpectNothing()
 }
 
 // Test the handler for 324 / RPL_CHANNELMODEIS
 func Test324(t *testing.T) {
-	m, c := setUp(t)
-	defer tearDown(m, c)
+	c, s := setUp(t)
+	defer s.tearDown()
 
 	// Create #test1, whose modes we don't know
-	test1 := c.NewChannel("#test1")
-	cm := test1.Modes
-
-	// Make sure modes are unset first
-	if cm.Secret || cm.NoExternalMsg || cm.Moderated || cm.Key != "" {
-		t.Errorf("Channel modes unexpectedly set before 324 reply.")
-	}
+	chan1 := state.NewChannel("#test1", s.log)
 
 	// Send a 324 reply
-	c.h_324(parseLine(":irc.server.org 324 test #test1 +snk somekey"))
-	m.ExpectNothing()
-
-	// Make sure the modes we expected to be set were set and vice versa
-	if !cm.Secret || !cm.NoExternalMsg || cm.Moderated || cm.Key != "somekey" {
-		t.Errorf("Channel modes unexpectedly set before 324 reply.")
+	s.st.EXPECT().GetChannel("#test1").Return(chan1)
+	c.h_324(parseLine(":irc.server.org 324 test #test1 +sk somekey"))
+	if !chan1.Modes.Secret || chan1.Modes.Key != "somekey" {
+		t.Errorf("Channel.ParseModes() not called correctly.")
 	}
 
 	// Check unknown channel causes an error
+	gomock.InOrder(
+		s.st.EXPECT().GetChannel("#test2").Return(nil),
+		s.log.EXPECT().Warn("irc.324(): received MODE settings for unknown "+
+			"channel %s", "#test2"),
+	)
 	c.h_324(parseLine(":irc.server.org 324 test #test2 +pmt"))
-	m.ExpectNothing()
 }
 
 // Test the handler for 332 / RPL_TOPIC
 func Test332(t *testing.T) {
-	m, c := setUp(t)
-	defer tearDown(m, c)
+	c, s := setUp(t)
+	defer s.tearDown()
 
 	// Create #test1, whose topic we don't know
-	test1 := c.NewChannel("#test1")
+	chan1 := state.NewChannel("#test1", s.log)
 
 	// Assert that it has no topic originally
-	if test1.Topic != "" {
+	if chan1.Topic != "" {
 		t.Errorf("Test channel already has a topic.")
 	}
 
 	// Send a 332 reply
+	s.st.EXPECT().GetChannel("#test1").Return(chan1)
 	c.h_332(parseLine(":irc.server.org 332 test #test1 :something something"))
-	m.ExpectNothing()
 
 	// Make sure the channel's topic has been changed
-	if test1.Topic != "something something" {
+	if chan1.Topic != "something something" {
 		t.Errorf("Topic of test channel not set correctly.")
 	}
 
 	// Check unknown channel causes an error
-	c.h_324(parseLine(":irc.server.org 332 test #test2 :dark side"))
-	m.ExpectNothing()
+	gomock.InOrder(
+		s.st.EXPECT().GetChannel("#test2").Return(nil),
+		s.log.EXPECT().Warn("irc.332(): received TOPIC value for unknown "+
+			"channel %s", "#test2"),
+	)
+	c.h_332(parseLine(":irc.server.org 332 test #test2 :dark side"))
 }
 
 // Test the handler for 352 / RPL_WHOREPLY
 func Test352(t *testing.T) {
-	m, c := setUp(t)
-	defer tearDown(m, c)
+	c, s := setUp(t)
+	defer s.tearDown()
 
 	// Create user1, who we know little about
-	user1 := c.NewNick("user1", "", "", "")
+	nick1 := state.NewNick("user1", s.log)
 
 	// Send a 352 reply
+	s.st.EXPECT().GetNick("user1").Return(nick1)
 	c.h_352(parseLine(":irc.server.org 352 test #test1 ident1 host1.com irc.server.org user1 G :0 name"))
-	m.ExpectNothing()
 
 	// Verify we now know more about user1
-	if user1.Ident != "ident1" ||
-		user1.Host != "host1.com" ||
-		user1.Name != "name" ||
-		user1.Modes.Invisible ||
-		user1.Modes.Oper {
+	if nick1.Ident != "ident1" ||
+		nick1.Host != "host1.com" ||
+		nick1.Name != "name" ||
+		nick1.Modes.Invisible ||
+		nick1.Modes.Oper {
 		t.Errorf("WHO info of user1 not set correctly.")
 	}
 
 	// Check that modes are set correctly from WHOREPLY
+	s.st.EXPECT().GetNick("user1").Return(nick1)
 	c.h_352(parseLine(":irc.server.org 352 test #test1 ident1 host1.com irc.server.org user1 H* :0 name"))
-	m.ExpectNothing()
 
-	if !user1.Modes.Invisible || !user1.Modes.Oper {
+	if !nick1.Modes.Invisible || !nick1.Modes.Oper {
 		t.Errorf("WHO modes of user1 not set correctly.")
 	}
 
 	// Check error paths -- send a 352 for an unknown nick
+	gomock.InOrder(
+		s.st.EXPECT().GetNick("user2").Return(nil),
+		s.log.EXPECT().Warn("irc.352(): received WHO reply for unknown nick %s",
+			"user2"),
+	)
 	c.h_352(parseLine(":irc.server.org 352 test #test2 ident2 host2.com irc.server.org user2 G :0 fooo"))
-	m.ExpectNothing()
 }
 
 // Test the handler for 353 / RPL_NAMREPLY
 func Test353(t *testing.T) {
-	m, c := setUp(t)
-	defer tearDown(m, c)
+	c, s := setUp(t)
+	defer s.tearDown()
 
 	// Create #test1, whose user list we're mostly unfamiliar with
-	test1 := c.NewChannel("#test1")
-	user1 := c.NewNick("user1", "ident1", "name one", "host1.com")
-	test1.AddNick(user1)
-	test1.AddNick(c.Me)
+	chan1 := state.NewChannel("#test1", s.log)
 
-	// lazy lazy lazy ;-)
-	get := func(n string) *ChanPrivs {
-		if p, ok := test1.Nicks[c.GetNick(n)]; ok {
-			return p
-		}
-		return nil
+	// Create maps for testing -- this is what the mock ST calls will return
+	nicks := make(map[string]*state.Nick)
+	privs := make(map[string]*state.ChanPrivs)
+
+	nicks["test"] = c.Me
+	privs["test"] = new(state.ChanPrivs)
+
+	for _, n := range []string{"user1", "user2", "voice", "halfop",
+		"op", "admin", "owner"} {
+		nicks[n] = state.NewNick(n, s.log)
+		privs[n] = new(state.ChanPrivs)
 	}
 
-	// Verify the lack of nicks
-	if len(test1.Nicks) != 2 {
-		t.Errorf("Unexpected number of nicks in test channel before 353.")
+	// 353 handler is called twice, so GetChannel will be called twice
+	s.st.EXPECT().GetChannel("#test1").Return(chan1).Times(2)
+	gomock.InOrder(
+		// "test" is Me, i am known, and already on the channel
+		s.st.EXPECT().GetNick("test").Return(c.Me),
+		s.st.EXPECT().IsOn("#test1", "test").Return(privs["test"], true),
+		// user1 is known, but not on the channel, so should be associated
+		s.st.EXPECT().GetNick("user1").Return(nicks["user1"]),
+		s.st.EXPECT().IsOn("#test1", "user1").Return(nil, false),
+		s.st.EXPECT().Associate(chan1, nicks["user1"]).Return(privs["user1"]),
+	)
+	for _, n := range []string{"user2", "voice", "halfop", "op", "admin", "owner"} {
+		gomock.InOrder(
+			s.st.EXPECT().GetNick(n).Return(nil),
+			s.st.EXPECT().NewNick(n).Return(nicks[n]),
+			s.st.EXPECT().IsOn("#test1", n).Return(nil, false),
+			s.st.EXPECT().Associate(chan1, nicks[n]).Return(privs[n]),
+		)
 	}
 
-	// Verify that user1 isn't opped yet
-	if p := get("user1"); p == nil || p.Op {
-		t.Errorf("Unexpected permissions for user1 before 353.")
-	}
-
-	// Send a couple of names replies (complete with trailing space), expect no errors
+	// Send a couple of names replies (complete with trailing space)
 	c.h_353(parseLine(":irc.server.org 353 test = #test1 :test @user1 user2 +voice "))
 	c.h_353(parseLine(":irc.server.org 353 test = #test1 :%halfop @op &admin ~owner "))
-	m.ExpectNothing()
 
-	if len(test1.Nicks) != 8 {
-		t.Errorf("Unexpected number of nicks in test channel after 353.")
+	if p := privs["user2"]; p.Voice || p.HalfOp || p.Op || p.Admin || p.Owner {
+		t.Errorf("353 handler incorrectly set modes on nick.")
 	}
 
-	// TODO(fluffle): Testing side-effects is starting to get on my tits.
-	// As a result, this makes some assumptions about the implementation of
-	// h_353 that may or may not be valid in the future. Hopefully, I will have
-	// time to rewrite the nick / channel handling soon.
-	if p := get("user1"); p == nil || !p.Op {
-		t.Errorf("353 handler failed to op known nick user1.")
+	if !privs["user1"].Op || !privs["voice"].Voice || !privs["halfop"].HalfOp ||
+		!privs["op"].Op || !privs["admin"].Admin || !privs["owner"].Owner {
+		t.Errorf("353 handler failed to set correct modes for nicks.")
 	}
 
-	if p := get("user2"); p == nil || p.Voice || p.HalfOp || p.Op || p.Admin || p.Owner {
-		t.Errorf("353 handler set modes on new nick user2.")
-	}
-
-	if p := get("voice"); p == nil || !p.Voice {
-		t.Errorf("353 handler failed to parse voice correctly.")
-	}
-
-	if p := get("halfop"); p == nil || !p.HalfOp {
-		t.Errorf("353 handler failed to parse halfop correctly.")
-	}
-
-	if p := get("op"); p == nil || !p.Op {
-		t.Errorf("353 handler failed to parse op correctly.")
-	}
-
-	if p := get("admin"); p == nil || !p.Admin {
-		t.Errorf("353 handler failed to parse admin correctly.")
-	}
-
-	if p := get("owner"); p == nil || !p.Owner {
-		t.Errorf("353 handler failed to parse owner correctly.")
-	}
-
-	// Check unknown channel causes an error
-	c.h_324(parseLine(":irc.server.org 353 test = #test2 :test ~user3"))
-	m.ExpectNothing()
+	// Check unknown channel causes a warning
+	gomock.InOrder(
+		s.st.EXPECT().GetChannel("#test2").Return(nil),
+		s.log.EXPECT().Warn("irc.353(): received NAMES list for unknown "+
+			"channel %s", "#test2"),
+	)
+	c.h_353(parseLine(":irc.server.org 353 test = #test2 :test ~user3"))
 }
 
 // Test the handler for 671 (unreal specific)
 func Test671(t *testing.T) {
-	m, c := setUp(t)
-	defer tearDown(m, c)
+	c, s := setUp(t)
+	defer s.tearDown()
 
 	// Create user1, who should not be secure
-	user1 := c.NewNick("user1", "ident1", "name one", "host1.com")
-	if user1.Modes.SSL {
+	nick1 := state.NewNick("user1", s.log)
+	if nick1.Modes.SSL {
 		t.Errorf("Test nick user1 is already using SSL?")
 	}
 
 	// Send a 671 reply
+	s.st.EXPECT().GetNick("user1").Return(nick1)
 	c.h_671(parseLine(":irc.server.org 671 test user1 :some ignored text"))
-	m.ExpectNothing()
 
 	// Ensure user1 is now known to be on an SSL connection
-	if !user1.Modes.SSL {
+	if !nick1.Modes.SSL {
 		t.Errorf("Test nick user1 not using SSL?")
 	}
 
 	// Check error paths -- send a 671 for an unknown nick
+	gomock.InOrder(
+		s.st.EXPECT().GetNick("user2").Return(nil),
+		s.log.EXPECT().Warn("irc.671(): received WHOIS SSL info for unknown "+
+			"nick %s", "user2"),
+	)
 	c.h_671(parseLine(":irc.server.org 671 test user2 :some ignored text"))
-	m.ExpectNothing()
 }