Skip to content

Commit 971b5bc

Browse files
schzhnschzhn
committed
all: imp code
1 parent 4fc73dc commit 971b5bc

File tree

10 files changed

+92
-72
lines changed

10 files changed

+92
-72
lines changed

‎internal/agh/agh.go

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ import (
1111
"github.com/AdguardTeam/golibs/testutil/fakeos/fakeexec"
1212
)
1313

14+
// DefaultOutputLimit is the default limit of bytes for commands' standard
15+
// output and standard error.
16+
const DefaultOutputLimit = 512
17+
1418
// ConfigModifier defines an interface for updating the global configuration.
1519
type ConfigModifier interface {
1620
// Apply applies changes to the global configuration.
@@ -27,37 +31,46 @@ var _ ConfigModifier = EmptyConfigModifier{}
2731
// Apply implements the [ConfigModifier] for EmptyConfigModifier.
2832
func (em EmptyConfigModifier) Apply(ctx context.Context) {}
2933

30-
// TODO(s.chzhen): !! Is there another way?
31-
//
32-
// TODO(s.chzhen): !! Docs, naming.
33-
//
34-
// TODO(s.chzhen): Move to aghtest once the import cycle is resolved.
34+
// exitErr implements [executil.ExitCodeError] for tests to simulate non-zero
35+
// process exit codes.
3536
type exitErr struct {
3637
code osutil.ExitCode
3738
}
3839

3940
// type check
4041
var _ executil.ExitCodeError = exitErr{}
4142

43+
// Error implements the [executil.ExitCodeError] for exitErr.
4244
func (e exitErr) Error() (s string) {
4345
return fmt.Sprintf("exit code %d", e.code)
4446
}
4547

48+
// ExitCode implements the [executil.ExitCodeError] for exitErr.
4649
func (e exitErr) ExitCode() (code osutil.ExitCode) {
4750
return e.code
4851
}
4952

53+
// ExternalCommand is a fake command used by [NewMultipleCommandConstructor].
5054
type ExternalCommand struct {
51-
Err error
52-
Cmd string
53-
Out string
55+
// Err is the error returned, if non-nil.
56+
Err error
57+
58+
// Cmd contains the command path and arguments.
59+
Cmd string
60+
61+
// Out is written to stdout if non-empty.
62+
Out string
63+
64+
// Code is returned as the exit code if non-zero.
5465
Code int
5566
}
5667

68+
// keyCommand builds a key for a command lookup.
5769
func keyCommand(path string, args []string) (k string) {
5870
return path + " " + strings.Join(args, " ")
5971
}
6072

73+
// parseCommand splits a command string into the executable path and args.
6174
func parseCommand(s string) (path string, args []string) {
6275
f := strings.Fields(s)
6376
if len(f) == 0 {
@@ -68,7 +81,11 @@ func parseCommand(s string) (path string, args []string) {
6881
}
6982

7083
// NewMultipleCommandConstructor is a helper function that returns a mock
71-
// [executil.CommandConstructor] for tests.
84+
// [executil.CommandConstructor] for tests that supports multiple commands.
85+
//
86+
// TODO(s.chzhen): Use this.
87+
//
88+
// TODO(s.chzhen): Move to aghtest once the import cycle is resolved.
7289
func NewMultipleCommandConstructor(cmds ...ExternalCommand) (cs executil.CommandConstructor) {
7390
table := make(map[string]ExternalCommand, len(cmds))
7491
for _, ec := range cmds {

‎internal/aghos/os.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,11 @@ func RunCommand(
9393
}
9494

9595
// psArgs holds the default ps arguments to avoid per-call slice allocations.
96+
//
97+
// Don't use -C flag here since it's a feature of linux's ps
98+
// implementation. Use POSIX-compatible flags instead.
99+
//
100+
// See https://github.com/AdguardTeam/AdGuardHome/issues/3457.
96101
var psArgs = []string{"-A", "-o", "pid=", "-o", "comm="}
97102

98103
// PIDByCommand searches for process named command and returns its PID ignoring
@@ -108,10 +113,6 @@ func PIDByCommand(
108113

109114
l.DebugContext(ctx, "executing", "cmd", psCmd, "args", psArgs)
110115

111-
// Don't use -C flag here since it's a feature of linux's ps
112-
// implementation. Use POSIX-compatible flags instead.
113-
//
114-
// See https://github.com/AdguardTeam/AdGuardHome/issues/3457.
115116
stdoutBuf := bytes.Buffer{}
116117
err = executil.Run(
117118
ctx,

‎internal/arpdb/arpdb.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"github.com/AdguardTeam/golibs/netutil"
1818
"github.com/AdguardTeam/golibs/osutil"
1919
"github.com/AdguardTeam/golibs/osutil/executil"
20+
"github.com/AdguardTeam/golibs/service"
2021
)
2122

2223
// Variables and functions to substitute in tests.
@@ -28,8 +29,8 @@ var (
2829
// Interface stores and refreshes the network neighborhood reported by ARP
2930
// (Address Resolution Protocol).
3031
type Interface interface {
31-
// Refresh updates the stored data. It must be safe for concurrent use.
32-
Refresh() (err error)
32+
// Refresher updates the stored data. It must be safe for concurrent use.
33+
service.Refresher
3334

3435
// Neighbors returnes the last set of data reported by ARP. Both the method
3536
// and it's result must be safe for concurrent use.
@@ -49,7 +50,7 @@ var _ Interface = Empty{}
4950

5051
// Refresh implements the [Interface] interface for EmptyARPContainer. It does
5152
// nothing and always returns nil error.
52-
func (Empty) Refresh() (err error) { return nil }
53+
func (Empty) Refresh(_ context.Context) (err error) { return nil }
5354

5455
// Neighbors implements the [Interface] interface for EmptyARPContainer. It
5556
// always returns nil.
@@ -174,13 +175,12 @@ type cmdARPDB struct {
174175
var _ Interface = (*cmdARPDB)(nil)
175176

176177
// Refresh implements the [Interface] interface for *cmdARPDB.
177-
func (arp *cmdARPDB) Refresh() (err error) {
178+
func (arp *cmdARPDB) Refresh(ctx context.Context) (err error) {
178179
defer func() { err = errors.Annotate(err, "cmd arpdb: %w") }()
179180

180181
var stdout bytes.Buffer
181182
err = executil.Run(
182-
// TODO(s.chzhen): Pass context.
183-
context.TODO(),
183+
ctx,
184184
arp.cmdCons,
185185
&executil.CommandConfig{
186186
Path: arp.cmd,
@@ -237,11 +237,11 @@ func newARPDBs(arps ...Interface) (arp *arpdbs) {
237237
var _ Interface = (*arpdbs)(nil)
238238

239239
// Refresh implements the [Interface] interface for *arpdbs.
240-
func (arp *arpdbs) Refresh() (err error) {
240+
func (arp *arpdbs) Refresh(ctx context.Context) (err error) {
241241
var errs []error
242242

243243
for _, a := range arp.arps {
244-
err = a.Refresh()
244+
err = a.Refresh(ctx)
245245
if err != nil {
246246
errs = append(errs, err)
247247

‎internal/arpdb/arpdb_internal_test.go

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
package arpdb
22

33
import (
4+
"context"
45
"io/fs"
56
"net"
67
"net/netip"
78
"os"
89
"sync"
910
"testing"
11+
"time"
1012

1113
"github.com/AdguardTeam/AdGuardHome/internal/agh"
1214
"github.com/AdguardTeam/golibs/errors"
@@ -16,6 +18,9 @@ import (
1618
"github.com/stretchr/testify/require"
1719
)
1820

21+
// testTimeout is a common timeout for tests.
22+
const testTimeout = 1 * time.Second
23+
1924
// testdata is the filesystem containing data for testing the package.
2025
var testdata fs.FS = os.DirFS("./testdata")
2126

@@ -33,16 +38,16 @@ func Test_New(t *testing.T) {
3338

3439
// TestARPDB is the mock implementation of [Interface] to use in tests.
3540
type TestARPDB struct {
36-
OnRefresh func() (err error)
41+
OnRefresh func(ctx context.Context) (err error)
3742
OnNeighbors func() (ns []Neighbor)
3843
}
3944

4045
// type check
4146
var _ Interface = (*TestARPDB)(nil)
4247

4348
// Refresh implements the [Interface] interface for *TestARPDB.
44-
func (arp *TestARPDB) Refresh() (err error) {
45-
return arp.OnRefresh()
49+
func (arp *TestARPDB) Refresh(ctx context.Context) (err error) {
50+
return arp.OnRefresh(ctx)
4651
}
4752

4853
// Neighbors implements the [Interface] interface for *TestARPDB.
@@ -60,21 +65,25 @@ func Test_NewARPDBs(t *testing.T) {
6065
}
6166

6267
succDB := &TestARPDB{
63-
OnRefresh: func() (err error) { succRefrCount++; return nil },
68+
OnRefresh: func(_ context.Context) (err error) { succRefrCount++; return nil },
6469
OnNeighbors: func() (ns []Neighbor) {
6570
return []Neighbor{{Name: "abc", IP: knownIP, MAC: knownMAC}}
6671
},
6772
}
6873
failDB := &TestARPDB{
69-
OnRefresh: func() (err error) { failRefrCount++; return errors.Error("refresh failed") },
74+
OnRefresh: func(_ context.Context) (err error) {
75+
failRefrCount++
76+
77+
return errors.Error("refresh failed")
78+
},
7079
OnNeighbors: func() (ns []Neighbor) { return nil },
7180
}
7281

7382
t.Run("begin_with_success", func(t *testing.T) {
7483
t.Cleanup(clnp)
7584

7685
a := newARPDBs(succDB, failDB)
77-
err := a.Refresh()
86+
err := a.Refresh(testutil.ContextWithTimeout(t, testTimeout))
7887
require.NoError(t, err)
7988

8089
assert.Equal(t, 1, succRefrCount)
@@ -86,7 +95,7 @@ func Test_NewARPDBs(t *testing.T) {
8695
t.Cleanup(clnp)
8796

8897
a := newARPDBs(failDB, succDB)
89-
err := a.Refresh()
98+
err := a.Refresh(testutil.ContextWithTimeout(t, testTimeout))
9099
require.NoError(t, err)
91100

92101
assert.Equal(t, 1, succRefrCount)
@@ -100,7 +109,7 @@ func Test_NewARPDBs(t *testing.T) {
100109
wantMsg := "each arpdb failed: refresh failed\nrefresh failed"
101110

102111
a := newARPDBs(failDB, failDB)
103-
err := a.Refresh()
112+
err := a.Refresh(testutil.ContextWithTimeout(t, testTimeout))
104113
require.Error(t, err)
105114

106115
testutil.AssertErrorMsg(t, wantMsg, err)
@@ -114,7 +123,7 @@ func Test_NewARPDBs(t *testing.T) {
114123

115124
shouldFail := false
116125
unstableDB := &TestARPDB{
117-
OnRefresh: func() (err error) {
126+
OnRefresh: func(_ context.Context) (err error) {
118127
if shouldFail {
119128
err = errors.Error("unstable failed")
120129
}
@@ -133,21 +142,21 @@ func Test_NewARPDBs(t *testing.T) {
133142
a := newARPDBs(unstableDB, succDB)
134143

135144
// Unstable ARPDB should refresh successfully.
136-
err := a.Refresh()
145+
err := a.Refresh(testutil.ContextWithTimeout(t, testTimeout))
137146
require.NoError(t, err)
138147

139148
assert.Zero(t, succRefrCount)
140149
assert.NotEmpty(t, a.Neighbors())
141150

142151
// Unstable ARPDB should fail and the succDB should be used.
143-
err = a.Refresh()
152+
err = a.Refresh(testutil.ContextWithTimeout(t, testTimeout))
144153
require.NoError(t, err)
145154

146155
assert.Equal(t, 1, succRefrCount)
147156
assert.NotEmpty(t, a.Neighbors())
148157

149158
// Unstable ARPDB should refresh successfully again.
150-
err = a.Refresh()
159+
err = a.Refresh(testutil.ContextWithTimeout(t, testTimeout))
151160
require.NoError(t, err)
152161

153162
assert.Equal(t, 1, succRefrCount)
@@ -156,7 +165,7 @@ func Test_NewARPDBs(t *testing.T) {
156165

157166
t.Run("empty", func(t *testing.T) {
158167
a := newARPDBs()
159-
require.NoError(t, a.Refresh())
168+
require.NoError(t, a.Refresh(testutil.ContextWithTimeout(t, testTimeout)))
160169

161170
assert.Empty(t, a.Neighbors())
162171
})
@@ -176,7 +185,7 @@ func TestCmdARPDB_arpa(t *testing.T) {
176185
t.Run("arp_a", func(t *testing.T) {
177186
a.cmdCons = agh.NewCommandConstructor("cmd", 0, arpAOutput, nil)
178187

179-
err := a.Refresh()
188+
err := a.Refresh(testutil.ContextWithTimeout(t, testTimeout))
180189
require.NoError(t, err)
181190

182191
assert.Equal(t, wantNeighs, a.Neighbors())
@@ -185,14 +194,14 @@ func TestCmdARPDB_arpa(t *testing.T) {
185194
t.Run("runcmd_error", func(t *testing.T) {
186195
a.cmdCons = agh.NewCommandConstructor("cmd", 0, "", errors.Error("can't run"))
187196

188-
err := a.Refresh()
197+
err := a.Refresh(testutil.ContextWithTimeout(t, testTimeout))
189198
testutil.AssertErrorMsg(t, "cmd arpdb: running command: running: can't run", err)
190199
})
191200

192201
t.Run("bad_code", func(t *testing.T) {
193202
a.cmdCons = agh.NewCommandConstructor("cmd", 1, "", nil)
194203

195-
err := a.Refresh()
204+
err := a.Refresh(testutil.ContextWithTimeout(t, testTimeout))
196205
testutil.AssertErrorMsg(
197206
t,
198207
"cmd arpdb: running command: unexpected exit code 1",
@@ -203,7 +212,7 @@ func TestCmdARPDB_arpa(t *testing.T) {
203212
t.Run("empty", func(t *testing.T) {
204213
a.cmdCons = agh.NewCommandConstructor("cmd", 0, "", nil)
205214

206-
err := a.Refresh()
215+
err := a.Refresh(testutil.ContextWithTimeout(t, testTimeout))
207216
require.NoError(t, err)
208217

209218
assert.Empty(t, a.Neighbors())
@@ -216,7 +225,7 @@ func TestEmptyARPDB(t *testing.T) {
216225
t.Run("refresh", func(t *testing.T) {
217226
var err error
218227
require.NotPanics(t, func() {
219-
err = a.Refresh()
228+
err = a.Refresh(testutil.ContextWithTimeout(t, testTimeout))
220229
})
221230

222231
assert.NoError(t, err)

‎internal/arpdb/arpdb_linux.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ package arpdb
44

55
import (
66
"bufio"
7+
"context"
78
"fmt"
89
"io/fs"
910
"log/slog"
@@ -76,7 +77,7 @@ type fsysARPDB struct {
7677
var _ Interface = (*fsysARPDB)(nil)
7778

7879
// Refresh implements the [Interface] interface for *fsysARPDB.
79-
func (arp *fsysARPDB) Refresh() (err error) {
80+
func (arp *fsysARPDB) Refresh(_ context.Context) (err error) {
8081
var f fs.File
8182
f, err = arp.fsys.Open(arp.filename)
8283
if err != nil {

‎internal/arpdb/arpdb_linux_internal_test.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111

1212
"github.com/AdguardTeam/AdGuardHome/internal/agh"
1313
"github.com/AdguardTeam/golibs/logutil/slogutil"
14+
"github.com/AdguardTeam/golibs/testutil"
1415
"github.com/stretchr/testify/assert"
1516
"github.com/stretchr/testify/require"
1617
)
@@ -55,7 +56,7 @@ func TestFSysARPDB(t *testing.T) {
5556
filename: "proc_net_arp",
5657
}
5758

58-
err := a.Refresh()
59+
err := a.Refresh(testutil.ContextWithTimeout(t, testTimeout))
5960
require.NoError(t, err)
6061

6162
ns := a.Neighbors()
@@ -76,7 +77,7 @@ func TestCmdARPDB_linux(t *testing.T) {
7677
},
7778
}
7879

79-
err := a.Refresh()
80+
err := a.Refresh(testutil.ContextWithTimeout(t, testTimeout))
8081
require.NoError(t, err)
8182

8283
assert.Equal(t, wantNeighs, a.Neighbors())
@@ -94,7 +95,7 @@ func TestCmdARPDB_linux(t *testing.T) {
9495
ns: make([]Neighbor, 0),
9596
},
9697
}
97-
err := a.Refresh()
98+
err := a.Refresh(testutil.ContextWithTimeout(t, testTimeout))
9899
require.NoError(t, err)
99100

100101
assert.Equal(t, wantNeighs, a.Neighbors())

‎internal/client/storage.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ func (s *Storage) addFromSystemARP(ctx context.Context) {
249249
s.mu.Lock()
250250
defer s.mu.Unlock()
251251

252-
if err := s.arpDB.Refresh(); err != nil {
252+
if err := s.arpDB.Refresh(ctx); err != nil {
253253
s.arpDB = arpdb.Empty{}
254254
s.logger.ErrorContext(ctx, "refreshing arp container", slogutil.KeyError, err)
255255

0 commit comments

Comments
 (0)