tests: add tests for reliable service under packet loss (#63)

Exercise reliable service passing a vector of packet ID to simulate
packet loss.

Additionally, this PR updates github actions version to the latest
action release for checkout (v4) and setup-go (v5). Eventually, we'll
currently replace the test setup with the one we're using to run tests
on the internal packages.

Co-authored-by: Simone Basso <bassosimone@gmail.com>

.github/workflows/build-refactor.yml

@@ -13,21 +13,30 @@ jobs:
   short-tests:
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v2
+    - uses: actions/checkout@v4
     - name: setup go
-      uses: actions/setup-go@v2
+      uses: actions/setup-go@v5
       with:
         go-version: '1.21'
     - name: Run short tests
       run: go test --short -cover ./internal/...
 
+  lint:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v4
+    - name: Lint with revive action, from pre-built image
+      uses: docker://morphy/revive-action:v2
+      with:
+        path: "internal/..."
+
   gosec:
     runs-on: ubuntu-latest
     env:
         GO111MODULE: on
     steps:
     - name: Checkout Source
-      uses: actions/checkout@v2
+      uses: actions/checkout@v4
     - name: Run Gosec security scanner
       uses: securego/gosec@master
       with:
@@ -36,9 +45,9 @@ jobs:
   coverage-threshold:
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v2
+    - uses: actions/checkout@v4
     - name: setup go
-      uses: actions/setup-go@v2
+      uses: actions/setup-go@v5
       with:
         go-version: '1.21'
     - name: Ensure coverage threshold
@@ -47,9 +56,9 @@ jobs:
   integration:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
       - name: setup go
-        uses: actions/setup-go@v2
+        uses: actions/setup-go@v5
         with:
           go-version: '1.21'
       - name: run integration tests

Makefile

@@ -99,16 +99,19 @@ netns-shell:
 	sudo ip netns exec protected sudo -u `whoami` -i
 
 .PHONY: lint
-lint: go-fmt go-vet go-sec
+lint: go-fmt go-vet go-sec go-revive
 
 go-fmt:
 	gofmt -s -l .
 
 go-vet:
-	go vet ./...
+	go vet internal/...
 
 go-sec:
-	gosec ./...
+	gosec internal/...
+
+go-revive:
+	revive internal/...
 
 clean:
 	@rm -f coverage.out

internal/reliabletransport/packets.go

@@ -33,11 +33,11 @@ func newInFlightPacket(p *model.Packet) *inFlightPacket {
 
 // ACKForHigherPacket increments the number of acks received for a higher pid than this packet. This will influence the fast rexmit selection algorithm.
 func (p *inFlightPacket) ACKForHigherPacket() {
-	p.higherACKs += 1
+	p.higherACKs++
 }
 
 func (p *inFlightPacket) ScheduleForRetransmission(t time.Time) {
-	p.retries += 1
+	p.retries++
 	p.deadline = t.Add(p.backoff())
 }
 

internal/reliabletransport/receiver.go

@@ -135,7 +135,7 @@ func (r *reliableReceiver) NextIncomingSequence() incomingSequence {
 	for i, p := range r.incomingPackets {
 		if p.ID-last == 1 {
 			ready = append(ready, p)
-			last += 1
+			last++
 		} else if p.ID > last {
 			// here we broke sequentiality, but we want
 			// to drop anything that is below lastConsumed

internal/reliabletransport/receiver_test.go

@@ -26,7 +26,9 @@ func Test_newReliableReceiver(t *testing.T) {
 }
 
 func Test_reliableQueue_MaybeInsertIncoming(t *testing.T) {
-	log.SetLevel(log.DebugLevel)
+	if testing.Verbose() {
+		log.SetLevel(log.DebugLevel)
+	}
 
 	type fields struct {
 		incomingPackets incomingSequence
@@ -91,7 +93,9 @@ func Test_reliableQueue_MaybeInsertIncoming(t *testing.T) {
 }
 
 func Test_reliableQueue_NextIncomingSequence(t *testing.T) {
-	log.SetLevel(log.DebugLevel)
+	if testing.Verbose() {
+		log.SetLevel(log.DebugLevel)
+	}
 
 	type fields struct {
 		lastConsumed    model.PacketID

internal/reliabletransport/reliable_ack_test.go

@@ -11,9 +11,21 @@ import (
 )
 
 // test that everything that is received from below is eventually ACKed to the sender.
+/*
+
+   ┌────┐id ┌────┐
+   │sndr│◄──┤rcvr│
+   └─┬──┘   └──▲─┘
+     │         │
+     │         │
+     │         │
+     ▼       send
+    ack
+*/
 func TestReliable_ACK(t *testing.T) {
-
-	log.SetLevel(log.DebugLevel)
+	if testing.Verbose() {
+		log.SetLevel(log.DebugLevel)
+	}
 
 	type args struct {
 		inputSequence []string
@@ -91,22 +103,6 @@ func TestReliable_ACK(t *testing.T) {
 				wantacks: 5,
 			},
 		},
-		/*
-			{
-				name: "a burst of packets",
-				args: args{
-					inputSequence: []string{
-						"[5] CONTROL_V1 +1ms",
-						"[1] CONTROL_V1 +1ms",
-						"[3] CONTROL_V1 +1ms",
-						"[2] CONTROL_V1 +1ms",
-						"[4] CONTROL_V1 +1ms",
-					},
-					start:    1,
-					wantacks: 5,
-				},
-			},
-		*/
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -148,7 +144,7 @@ func TestReliable_ACK(t *testing.T) {
 			reader := vpntest.NewPacketReader(dataOut)
 			witness := vpntest.NewWitness(reader)
 
-			if ok := witness.VerifyNumberOfACKs(tt.args.start, tt.args.wantacks, t0); !ok {
+			if ok := witness.VerifyNumberOfACKs(tt.args.wantacks, t0); !ok {
 				got := len(witness.Log().ACKs())
 				t.Errorf("TestACK: got = %v, want %v", got, tt.args.wantacks)
 			}