pieceordering: Rename RemovePiece->DeletePiece and complete tests

Note that tests currently fail due to bug in skiplist?

client.go

@@ -2019,7 +2019,7 @@ func (me *Client) downloadedChunk(t *torrent, c *connection, msg *pp.Message) er
 	me.dataReady(t, req)
 	if len(t.Pieces[req.Index].PendingChunkSpecs) == 0 {
 		for _, c := range t.Conns {
-			c.pieceRequestOrder.RemovePiece(int(req.Index))
+			c.pieceRequestOrder.DeletePiece(int(req.Index))
 		}
 		me.queuePieceCheck(t, req.Index)
 	}
@@ -2117,7 +2117,7 @@ func (me *Client) pieceHashed(t *torrent, piece pp.Integer, correct bool) {
 					panic("wat")
 				}
 			}
-			conn.pieceRequestOrder.RemovePiece(int(piece))
+			conn.pieceRequestOrder.DeletePiece(int(piece))
 		}
 		if t.wantPiece(int(piece)) && conn.PeerHasPiece(piece) {
 			conn.pendPiece(int(piece), t.Pieces[piece].Priority)

connection.go

@@ -89,7 +89,7 @@ func newConnection(sock net.Conn, peb peerExtensionBytes, peerID [20]byte, uTP b
 
 func (cn *connection) pendPiece(piece int, priority piecePriority) {
 	if priority == piecePriorityNone {
-		cn.pieceRequestOrder.RemovePiece(piece)
+		cn.pieceRequestOrder.DeletePiece(piece)
 		return
 	}
 	key := cn.piecePriorities[piece]

internal/pieceordering/pieceordering.go

@@ -22,9 +22,7 @@ func (me *Instance) SetPiece(piece, key int) {
 		if existingKey == key {
 			return
 		}
-		if me.sl.Remove(existingKey).Value.(int) != piece {
-			panic("piecekeys map lied to us")
-		}
+		me.removeKeyPiece(existingKey, piece)
 	}
 	me.sl.Insert(key, piece)
 	if me.pieceKeys == nil {
@@ -33,18 +31,21 @@ func (me *Instance) SetPiece(piece, key int) {
 	me.pieceKeys[piece] = key
 }
 
-func (me *Instance) RemovePiece(piece int) {
-	key, ok := me.pieceKeys[piece]
-	if !ok {
-		return
-	}
-	el := me.sl.Remove(key)
-	if el == nil {
-		panic("element not present but should be")
+func (me *Instance) removeKeyPiece(key, piece int) {
+	if me.sl.Remove(key).Value.(int) != piece {
+		panic("piecekeys map lied to us")
 	}
 	if me.sl.Remove(key) != nil {
 		panic("duplicate key")
 	}
+}
+
+func (me *Instance) DeletePiece(piece int) {
+	key, ok := me.pieceKeys[piece]
+	if !ok {
+		return
+	}
+	me.removeKeyPiece(key, piece)
 	delete(me.pieceKeys, piece)
 }
 

internal/pieceordering/pieceordering_test.go

@@ -4,16 +4,26 @@ import (
 	"testing"
 )
 
+func instanceSlice(i *Instance) (sl []int) {
+	for e := i.First(); e != nil; e = e.Next() {
+		sl = append(sl, e.Piece())
+	}
+	return
+}
+
 func checkOrder(t *testing.T, i *Instance, pp []int) {
+	fatal := func() {
+		t.Fatalf("have %v, expected %v", instanceSlice(i), pp)
+	}
 	e := i.First()
 	for _, p := range pp {
 		if p != e.Piece() {
-			t.FailNow()
+			fatal()
 		}
 		e = e.Next()
 	}
 	if e != nil {
-		t.FailNow()
+		fatal()
 	}
 }
 
@@ -24,10 +34,26 @@ func TestPieceOrdering(t *testing.T) {
 	checkOrder(t, i, []int{1, 0})
 	i.SetPiece(1, 2)
 	checkOrder(t, i, []int{0, 1})
-	i.RemovePiece(1)
+	i.DeletePiece(1)
 	checkOrder(t, i, []int{0})
-	i.RemovePiece(2)
-	i.RemovePiece(1)
+	i.DeletePiece(2)
+	i.DeletePiece(1)
 	checkOrder(t, i, []int{0})
-	i.RemovePiece(0)
+	i.DeletePiece(0)
+	checkOrder(t, i, nil)
+	i.SetPiece(2, 1)
+	i.SetPiece(1, 1)
+	i.SetPiece(3, 1)
+	checkOrder(t, i, []int{3, 1, 2})
+	// Move a piece that isn't the youngest in a key.
+	i.SetPiece(1, -1)
+	checkOrder(t, i, []int{1, 3, 2})
+	i.DeletePiece(2)
+	i.DeletePiece(3)
+	i.DeletePiece(1)
+	checkOrder(t, i, nil)
+	i.DeletePiece(2)
+	i.DeletePiece(3)
+	i.DeletePiece(1)
+	checkOrder(t, i, nil)
 }