ronantakizawa/codereview-bench huggingface dataset:
Repository: moby/moby
File: libnetwork/drivers/bridge/bridge_linux.go (Go)

Code before:

	endpoint, err := network.getEndpoint(eid)
	if err != nil {
		return err
	}

	if endpoint == nil {
		return endpointNotFoundError(eid)
	}

	if !network.config.EnableICC {
		if err = d.link(network, endpoint, false); err != nil {
			return err
		}
	}

	return nil
}

func (d *driver) ProgramExternalConnectivity(ctx context.Context, nid, eid string, options map[string]interface{}) error {
	ctx, span := otel.Tracer("").Start(ctx, "libnetwork.drivers.bridge.ProgramExternalConnectivity", trace.WithAttributes(
		attribute.String("nid", nid),
		attribute.String("eid", eid)))
	defer span.End()

	// Make sure the network isn't deleted, or the in middle of a firewalld reload, while
	// updating its iptables rules.
	d.configNetwork.Lock()
	defer d.configNetwork.Unlock()

	network, err := d.getNetwork(nid)
	if err != nil {
		return err
	}

	endpoint, err := network.getEndpoint(eid)
	if err != nil {
		return err
	}

	if endpoint == nil {
		return endpointNotFoundError(eid)
	}

	endpoint.extConnConfig, err = parseConnectivityOptions(options)
	if err != nil {
		return err
	}

	// Program any required port mapping and store them in the endpoint
	if endpoint.extConnConfig != nil && endpoint.extConnConfig.PortBindings != nil {

Review comment (suggestion):
```suggestion
	// Make sure the network isn't deleted, or in the middle of a firewalld reload, while
```

Code after:
	defer span.End()

	// Make sure the network isn't deleted, or in the middle of a firewalld reload, while
	// updating its iptables rules.
	d.configNetwork.Lock()
	defer d.configNetwork.Unlock()

	network, err := d.getNetwork(nid)
	if err != nil {
		return err
	}

	endpoint, err := network.getEndpoint(eid)
	if err != nil {
		return err
	}

	if endpoint == nil {
		return endpointNotFoundError(eid)
	}

	endpoint.extConnConfig, err = parseConnectivityOptions(options)
	if err != nil {
		return err
	}

	// Program any required port mapping and store them in the endpoint
	if endpoint.extConnConfig != nil && endpoint.extConnConfig.PortBindings != nil {
		endpoint.portMapping, err = network.addPortMappings(
			ctx,
			endpoint.addr,
			endpoint.addrv6,
			endpoint.extConnConfig.PortBindings,
			network.config.DefaultBindingIP,
			endpoint.extConnConfig.NoProxy6To4,
		)
		if err != nil {
			return err
		}
	}

	defer func() {
		if err != nil {
			if e := network.releasePorts(endpoint); e != nil {
				log.G(ctx).Errorf("Failed to release ports allocated for the bridge endpoint %s on failure %v because of %v",
					eid, err, e)
			}
			endpoint.portMapping = nil
		}
	}()
Repository: moby/moby
File: libnetwork/drivers/bridge/bridge_linux.go (Go)

Code before:
		if err != nil {
			if e := network.releasePorts(endpoint); e != nil {
				log.G(ctx).Errorf("Failed to release ports allocated for the bridge endpoint %s on failure %v because of %v",
					eid, err, e)
			}
			endpoint.portMapping = nil
		}
	}()

	// Clean the connection tracker state of the host for the specific endpoint. This is needed because some flows may
	// be bound to the local proxy, or to the host (for UDP packets), and won't be redirected to the new endpoints.
	clearConntrackEntries(d.nlh, endpoint)

	if err = d.storeUpdate(ctx, endpoint); err != nil {
		return fmt.Errorf("failed to update bridge endpoint %.7s to store: %v", endpoint.id, err)
	}

	if !network.config.EnableICC {
		return d.link(network, endpoint, true)
	}

	return nil
}

func (d *driver) RevokeExternalConnectivity(nid, eid string) error {
	// Make sure this doesn't function isn't deleting iptables rules while handleFirewalldReloadNw
	// is restoring those same rules.
	d.configNetwork.Lock()
	defer d.configNetwork.Unlock()

	network, err := d.getNetwork(nid)
	if err != nil {
		return err
	}

	endpoint, err := network.getEndpoint(eid)
	if err != nil {
		return err
	}

	if endpoint == nil {
		return endpointNotFoundError(eid)
	}

	err = network.releasePorts(endpoint)
	if err != nil {
		log.G(context.TODO()).Warn(err)
	}

	endpoint.portMapping = nil


Review comment (suggestion):
```suggestion
	// Make sure this function isn't deleting iptables rules while handleFirewalldReloadNw
```

Code after:

func (d *driver) RevokeExternalConnectivity(nid, eid string) error {
	// Make sure this function isn't deleting iptables rules while handleFirewalldReloadNw
	// is restoring those same rules.
	d.configNetwork.Lock()
	defer d.configNetwork.Unlock()

	network, err := d.getNetwork(nid)
	if err != nil {
		return err
	}

	endpoint, err := network.getEndpoint(eid)
	if err != nil {
		return err
	}

	if endpoint == nil {
		return endpointNotFoundError(eid)
	}

	err = network.releasePorts(endpoint)
	if err != nil {
		log.G(context.TODO()).Warn(err)
	}

	endpoint.portMapping = nil

	// Clean the connection tracker state of the host for the specific endpoint. This is a precautionary measure to
	// avoid new endpoints getting the same IP address to receive unexpected packets due to bad conntrack state leading
	// to bad NATing.
	clearConntrackEntries(d.nlh, endpoint)

	if err = d.storeUpdate(context.TODO(), endpoint); err != nil {
		return fmt.Errorf("failed to update bridge endpoint %.7s to store: %v", endpoint.id, err)
	}

	return nil
}

func (d *driver) handleFirewalldReload() {
	if !d.config.EnableIPTables && !d.config.EnableIP6Tables {
		return
	}

	d.Lock()
	nids := make([]string, 0, len(d.networks))
	for _, nw := range d.networks {
		nids = append(nids, nw.id)
	}
	d.Unlock()
Repository: moby/moby
File: libnetwork/iptables/firewalld.go (Go)

Code before:
	if len(newOwner) > 0 {
		connectionEstablished()
	} else if len(oldOwner) > 0 {
		connectionLost()
	}
}

func connectionEstablished() {
	reloaded()
}

func connectionLost() {
	// Doesn't do anything for now. Libvirt also doesn't react to this.
}

// call all callbacks
func reloaded() {
	firewalldReloadMu.Lock()
	defer firewalldReloadMu.Unlock()
	for _, pf := range onReloaded {
		(*pf)()
	}

	firewalldReloadedAtMu.Lock()
	defer firewalldReloadedAtMu.Unlock()
	firewalldReloadedAt = time.Now()
}

// OnReloaded add callback
func OnReloaded(callback func()) {
	for _, pf := range onReloaded {
		if pf == &callback {
			return
		}
	}
	onReloaded = append(onReloaded, &callback)
}

// Call some remote method to see whether the service is actually running.
func checkRunning() bool {
	if connection == nil {
		return false
	}
	var zone string
	err := connection.sysObj.Call(dbusInterface+".getDefaultZone", 0).Store(&zone)
	return err == nil
}

// passthrough method simply passes args through to iptables/ip6tables
func passthrough(ipv IPVersion, args ...string) ([]byte, error) {
	var output string

Review comment (suggestion):
I was wondering if we  should use `time.Now().UTC()` here, but it looks like we don't do so for `SystemTime` (wondering if we should 🤔)

https://github.com/moby/moby/blob/26e516dff4384bf3bc2233020a2f5f4001755efb/daemon/info.go#L58


```suggestion
	firewalldReloadedAt.Store(time.Now())
```

Code after:
	}

	if len(newOwner) > 0 {
		connectionEstablished()
	} else if len(oldOwner) > 0 {
		connectionLost()
	}
}

func connectionEstablished() {
	reloaded()
}

func connectionLost() {
	// Doesn't do anything for now. Libvirt also doesn't react to this.
}

// call all callbacks
func reloaded() {
	firewalldReloadMu.Lock()
	defer firewalldReloadMu.Unlock()
	for _, pf := range onReloaded {
		(*pf)()
	}
	firewalldReloadedAt.Store(time.Now())
}

// OnReloaded add callback
func OnReloaded(callback func()) {
	for _, pf := range onReloaded {
		if pf == &callback {
			return
		}
	}
	onReloaded = append(onReloaded, &callback)
}

// Call some remote method to see whether the service is actually running.
func checkRunning() bool {
	if connection == nil {
		return false
	}
	var zone string
	err := connection.sysObj.Call(dbusInterface+".getDefaultZone", 0).Store(&zone)
	return err == nil
}

// passthrough method simply passes args through to iptables/ip6tables
func passthrough(ipv IPVersion, args ...string) ([]byte, error) {
	var output string
	log.G(context.TODO()).Debugf("Firewalld passthrough: %s, %s", ipv, args)
Repository: moby/moby
File: internal/testutils/networking/iptables.go (Go)

Code before:

func FirewalldRunning() bool {
	state, err := exec.Command("firewall-cmd", "--state").CombinedOutput()
	return err == nil && strings.TrimSpace(string(state)) == "running"
}

// FirewalldReload reloads firewalld and waits for the daemon to re-create its rules.
// It's a no-op if firewalld is not running, and the test fails if the reload does
// not complete.
func FirewalldReload(t *testing.T, d *daemon.Daemon) {
	t.Helper()
	if !FirewalldRunning() {
		return
	}
	lastReload := d.FirewallReloadedAt(t)
	res := icmd.RunCommand("firewall-cmd", "--reload")
	assert.NilError(t, res.Error)

	poll.WaitOn(t, func(_ poll.LogT) poll.Result {
		latestReload := d.FirewallReloadedAt(t)
		if latestReload != "" && latestReload != lastReload {
			t.Log("Firewalld reload completed at", latestReload)
			return poll.Success()
		}
		return poll.Continue("firewalld reload not complete")
	}, poll.WithDelay(100*time.Millisecond), poll.WithTimeout(20*time.Second))
}


Review comment (refactor):
FWIW; default is 100 miliseconds interval (so we don't need to set it), and 10 seconds timeout. If 10 seconds would still be enough for this one, we could remove this config.

(not a blocker for sure; mostly that I removed a bunch of these at some point where the custom options were the same as the default)

Code after:
func FirewalldRunning() bool {
	state, err := exec.Command("firewall-cmd", "--state").CombinedOutput()
	return err == nil && strings.TrimSpace(string(state)) == "running"
}

// FirewalldReload reloads firewalld and waits for the daemon to re-create its rules.
// It's a no-op if firewalld is not running, and the test fails if the reload does
// not complete.
func FirewalldReload(t *testing.T, d *daemon.Daemon) {
	t.Helper()
	if !FirewalldRunning() {
		return
	}
	lastReload := d.FirewallReloadedAt(t)
	res := icmd.RunCommand("firewall-cmd", "--reload")
	assert.NilError(t, res.Error)

	poll.WaitOn(t, func(_ poll.LogT) poll.Result {
		latestReload := d.FirewallReloadedAt(t)
		if latestReload != "" && latestReload != lastReload {
			t.Log("Firewalld reload completed at", latestReload)
			return poll.Success()
		}
		return poll.Continue("firewalld reload not complete")
	})
}
Repository: moby/moby
File: daemon/stats_unix_test.go (Go)

Code before:
package daemon

import (
	"os"
	"path/filepath"
	"testing"

	"gotest.tools/v3/assert"
)

func TestGetSystemCPUUsageParsing(t *testing.T) {
	dummyFilePath := filepath.Join("testdata", "stat")
	expectedCpuUsage := uint64(65647090000000)
	expectedCpuNum := uint32(128)

	origStatPath := procStatPath
	procStatPath = dummyFilePath
	defer func() { procStatPath = origStatPath }()

	_, err := os.Stat(dummyFilePath)
	assert.NilError(t, err)

	cpuUsage, cpuNum, err := getSystemCPUUsage()

	assert.Equal(t, cpuUsage, expectedCpuUsage)
	assert.Equal(t, cpuNum, expectedCpuNum)
	assert.NilError(t, err)
}

Review comment (suggestion):
Unit test is not buildable on non-unix platforms:
```
 === Errors
daemon\stats_unix_test.go:16:18: undefined: procStatPath
daemon\stats_unix_test.go:17:2: undefined: procStatPath
daemon\stats_unix_test.go:18:17: undefined: procStatPath
```

This is because Go doesn't support a `_unix` prefix natively and it's more of a convention.
So this still needs an explicit go:build flag:

```suggestion
//go:build !windows

package daemon
```

Code after:
//go:build !windows

package daemon

import (
	"os"
	"path/filepath"
	"testing"

	"gotest.tools/v3/assert"
)

func TestGetSystemCPUUsageParsing(t *testing.T) {
	dummyFilePath := filepath.Join("testdata", "stat")
	expectedCpuUsage := uint64(65647090000000)
	expectedCpuNum := uint32(128)

	origStatPath := procStatPath
	procStatPath = dummyFilePath
	defer func() { procStatPath = origStatPath }()

	_, err := os.Stat(dummyFilePath)
	assert.NilError(t, err)

	cpuUsage, cpuNum, err := getSystemCPUUsage()

	assert.Equal(t, cpuUsage, expectedCpuUsage)
	assert.Equal(t, cpuNum, expectedCpuNum)
Repository: moby/moby
File: daemon/stats_unix_test.go (Go)

Code before:
package daemon

import (
	"os"
	"path/filepath"
	"testing"

	"gotest.tools/v3/assert"
)

func TestGetSystemCPUUsageParsing(t *testing.T) {
	dummyFilePath := filepath.Join("testdata", "stat")
	expectedCpuUsage := uint64(65647090000000)
	expectedCpuNum := uint32(128)

	origStatPath := procStatPath
	procStatPath = dummyFilePath
	defer func() { procStatPath = origStatPath }()

	_, err := os.Stat(dummyFilePath)
	assert.NilError(t, err)

	cpuUsage, cpuNum, err := getSystemCPUUsage()

	assert.Equal(t, cpuUsage, expectedCpuUsage)
	assert.Equal(t, cpuNum, expectedCpuNum)
	assert.NilError(t, err)
}


Review comment (suggestion):
This way we can avoid replacing the file path and inline the testdata in the test:
```suggestion
	input := strings.NewReader(```
<stat content>
```)
```

Code after:
//go:build !windows

package daemon

import (
	"os"
	"path/filepath"
	"testing"

	"gotest.tools/v3/assert"
)

func TestGetSystemCPUUsageParsing(t *testing.T) {
	dummyFilePath := filepath.Join("testdata", "stat")
	expectedCpuUsage := uint64(65647090000000)
	expectedCpuNum := uint32(128)

	origStatPath := procStatPath
	procStatPath = dummyFilePath
	defer func() { procStatPath = origStatPath }()

	_, err := os.Stat(dummyFilePath)
	assert.NilError(t, err)

	cpuUsage, cpuNum, err := getSystemCPUUsage()

	assert.Equal(t, cpuUsage, expectedCpuUsage)
	assert.Equal(t, cpuNum, expectedCpuNum)
	assert.NilError(t, err)
}
Repository: moby/moby
File: daemon/stats_unix_test.go (Go)

Code before:
package daemon

import (
	"os"
	"path/filepath"
	"testing"

	"gotest.tools/v3/assert"
)

func TestGetSystemCPUUsageParsing(t *testing.T) {
	dummyFilePath := filepath.Join("testdata", "stat")
	expectedCpuUsage := uint64(65647090000000)
	expectedCpuNum := uint32(128)

	origStatPath := procStatPath
	procStatPath = dummyFilePath
	defer func() { procStatPath = origStatPath }()

	_, err := os.Stat(dummyFilePath)
	assert.NilError(t, err)

	cpuUsage, cpuNum, err := getSystemCPUUsage()

	assert.Equal(t, cpuUsage, expectedCpuUsage)
	assert.Equal(t, cpuNum, expectedCpuNum)
	assert.NilError(t, err)
}


Review comment (suggestion):
And then test the function that allows passing the file content directly:
```suggestion
	cpuUsage, cpuNum, err := readSystemCPUUsage(input)
```

Code after:
//go:build !windows

package daemon

import (
	"os"
	"path/filepath"
	"testing"

	"gotest.tools/v3/assert"
)

func TestGetSystemCPUUsageParsing(t *testing.T) {
	dummyFilePath := filepath.Join("testdata", "stat")
	expectedCpuUsage := uint64(65647090000000)
	expectedCpuNum := uint32(128)

	origStatPath := procStatPath
	procStatPath = dummyFilePath
	defer func() { procStatPath = origStatPath }()

	_, err := os.Stat(dummyFilePath)
	assert.NilError(t, err)

	cpuUsage, cpuNum, err := getSystemCPUUsage()

	assert.Equal(t, cpuUsage, expectedCpuUsage)
	assert.Equal(t, cpuNum, expectedCpuNum)
	assert.NilError(t, err)
}
Repository: moby/moby
File: daemon/stats_unix_test.go (Go)

Code before:
import (
	"os"
	"path/filepath"
	"testing"

	"gotest.tools/v3/assert"
)

func TestGetSystemCPUUsageParsing(t *testing.T) {
	dummyFilePath := filepath.Join("testdata", "stat")
	expectedCpuUsage := uint64(65647090000000)
	expectedCpuNum := uint32(128)

	origStatPath := procStatPath
	procStatPath = dummyFilePath
	defer func() { procStatPath = origStatPath }()

	_, err := os.Stat(dummyFilePath)
	assert.NilError(t, err)

	cpuUsage, cpuNum, err := getSystemCPUUsage()

	assert.Equal(t, cpuUsage, expectedCpuUsage)
	assert.Equal(t, cpuNum, expectedCpuNum)
	assert.NilError(t, err)
}


Review comment (suggestion):
We can inline `expectedCpuUsage` and `expectedCpuNum` and use `cmp` assertions:

```suggestion
	assert.Check(t, is.Equal(cpuUsage, uint64(65647090000000)))
	assert.Check(t, is.Equal(cpuNum, uint32(128)))
```

Code after:
package daemon

import (
	"os"
	"path/filepath"
	"testing"

	"gotest.tools/v3/assert"
)

func TestGetSystemCPUUsageParsing(t *testing.T) {
	dummyFilePath := filepath.Join("testdata", "stat")
	expectedCpuUsage := uint64(65647090000000)
	expectedCpuNum := uint32(128)

	origStatPath := procStatPath
	procStatPath = dummyFilePath
	defer func() { procStatPath = origStatPath }()

	_, err := os.Stat(dummyFilePath)
	assert.NilError(t, err)

	cpuUsage, cpuNum, err := getSystemCPUUsage()

	assert.Equal(t, cpuUsage, expectedCpuUsage)
	assert.Equal(t, cpuNum, expectedCpuNum)
	assert.NilError(t, err)
}
Repository: moby/moby
File: daemon/containerd/progress.go (Go)

Code before:
			Action:  "Pushing",
			Current: status.Offset,
			Total:   status.Total,
		})
	}

	return nil
}

type combinedProgress []progressUpdater

func (combined combinedProgress) UpdateProgress(ctx context.Context, ongoing *jobs, out progress.Output, start time.Time) error {
	for _, p := range combined {
		err := p.UpdateProgress(ctx, ongoing, out, start)
		if err != nil {
			return err
		}
	}
	return nil
}

func showBlobProgress(desc ocispec.Descriptor) bool {
	switch {
	case c8dimages.IsIndexType(desc.MediaType),
		c8dimages.IsManifestType(desc.MediaType),
		c8dimages.IsConfigType(desc.MediaType):
		return false
	default:
		return true
	}
}


Review comment (suggestion):
Perhaps it's worth adding a GoDoc to this function.

It's clear _what_ it does, but not _why_ it does; a short description that it's used to decide whether or not progress should be shown, and why we're considering specific types should not show it.

Code after:
			Action:  "Pushing",
			Current: status.Offset,
			Total:   status.Total,
		})
	}

	return nil
}

type combinedProgress []progressUpdater

func (combined combinedProgress) UpdateProgress(ctx context.Context, ongoing *jobs, out progress.Output, start time.Time) error {
	for _, p := range combined {
		err := p.UpdateProgress(ctx, ongoing, out, start)
		if err != nil {
			return err
		}
	}
	return nil
}

// showBlobProgress determines if the progress of pulling/pushing blob should be shown.
// Only indexes, manifests, and configs are hidden to align with the pre-containerd behavior.
// They are small enough JSON files so it's fine to not show them.
// We mostly care about bigger content like layers or other blobs.
func showBlobProgress(desc ocispec.Descriptor) bool {
	switch {
	case c8dimages.IsLayerType(desc.MediaType):
		// Fast path: we always show progress for layers.
		//
		// Note: We can't just plainly check for c8dimages.IsLayerType alone
		// because it wouldn't account for other potentially big blobs like
		// artifacts or non-standard images.
		return true
	case c8dimages.IsIndexType(desc.MediaType),
		c8dimages.IsManifestType(desc.MediaType),
		c8dimages.IsConfigType(desc.MediaType):
		return false
	default:
		return true
	}
}
Repository: moby/moby
File: daemon/containerd/progress.go (Go)

Code before:
		err := p.UpdateProgress(ctx, ongoing, out, start)
		if err != nil {
			return err
		}
	}
	return nil
}

// showBlobProgress determines if the progress of pulling/pushing blob should be shown.
// Only indexes, manifests, and configs are hidden to align with the pre-containerd behavior.
// They are small enough JSON files so it's fine to not show them.
// We mostly care about bigger content like layers or other blobs.
func showBlobProgress(desc ocispec.Descriptor) bool {
	// We can't just plainly use c8dimages.IsLayerType here because it wouldn't
	// account for other potentially big blobs like artifacts or non-standard
	// images.
	switch {
	case c8dimages.IsIndexType(desc.MediaType),
		c8dimages.IsManifestType(desc.MediaType),
		c8dimages.IsConfigType(desc.MediaType):
		return false
	default:
		return true
	}
}


Review comment (suggestion):
Thanks for that comment; silly question; would we still be able to take advantage of it as a fast-path? Considering;


```suggestion
	// We can't just plainly use c8dimages.IsLayerType here because it wouldn't
	// account for other potentially big blobs like artifacts or non-standard
	// images.
	switch {
	case c8dimages.IsLayerType(desc.MediaType):
		// Fast path: we always show progress for layers.	
		return true
	case c8dimages.IsIndexType(desc.MediaType),
		c8dimages.IsManifestType(desc.MediaType),
		c8dimages.IsConfigType(desc.MediaType):
		return false
	default:
```

Code after:
		err := p.UpdateProgress(ctx, ongoing, out, start)
		if err != nil {
			return err
		}
	}
	return nil
}

// showBlobProgress determines if the progress of pulling/pushing blob should be shown.
// Only indexes, manifests, and configs are hidden to align with the pre-containerd behavior.
// They are small enough JSON files so it's fine to not show them.
// We mostly care about bigger content like layers or other blobs.
func showBlobProgress(desc ocispec.Descriptor) bool {
	switch {
	case c8dimages.IsLayerType(desc.MediaType):
		// Fast path: we always show progress for layers.
		//
		// Note: We can't just plainly check for c8dimages.IsLayerType alone
		// because it wouldn't account for other potentially big blobs like
		// artifacts or non-standard images.
		return true
	case c8dimages.IsIndexType(desc.MediaType),
		c8dimages.IsManifestType(desc.MediaType),
		c8dimages.IsConfigType(desc.MediaType):
		return false
	default:
		return true
	}
}
Repository: moby/moby
File: libnetwork/endpoint_store_test.go (Go)

Code before:

func TestEndpointStore(t *testing.T) {
	configOption := config.OptionDataDir(t.TempDir())
	c, err := New(configOption)
	assert.NilError(t, err)
	defer c.Stop()

	// Insert a first endpoint
	nw := &Network{id: "testNetwork"}
	ep1 := &Endpoint{network: nw, id: "testEndpoint1"}
	err = c.storeEndpoint(context.Background(), ep1)
	assert.NilError(t, err)

	// Then a second endpoint
	ep2 := &Endpoint{network: nw, id: "testEndpoint2"}
	err = c.storeEndpoint(context.Background(), ep2)
	assert.NilError(t, err)

	epSorter := func(a, b *Endpoint) int {
		if a.id < b.id {
			return -1
		}
		if a.id > b.id {
			return 1
		}
		return 0
	}

	// Check that we can find both endpoints
	found := c.findEndpoints(filterEndpointByNetworkId("testNetwork"))
	slices.SortFunc(found, epSorter)
	assert.Equal(t, len(found), 2)
	assert.Equal(t, found[0], ep1)
	assert.Equal(t, found[1], ep2)

	// Delete the first endpoint
	err = c.deleteStoredEndpoint(ep1)
	assert.NilError(t, err)

	// Check that we can only find the second endpoint
	found = c.findEndpoints(filterEndpointByNetworkId("testNetwork"))
	assert.Equal(t, len(found), 1)
	assert.Equal(t, found[0], ep2)

	// Store the second endpoint again
	err = c.storeEndpoint(context.Background(), ep2)
	assert.NilError(t, err)
}


Review comment (suggestion):
Could use this? ...
```suggestion
		return strings.Compare(a.id, b.id)
```

Code after:
	is "gotest.tools/v3/assert/cmp"
)

func TestEndpointStore(t *testing.T) {
	configOption := config.OptionDataDir(t.TempDir())
	c, err := New(configOption)
	assert.NilError(t, err)
	defer c.Stop()

	// Insert a first endpoint
	nw := &Network{id: "testNetwork"}
	ep1 := &Endpoint{network: nw, id: "testEndpoint1"}
	err = c.storeEndpoint(context.Background(), ep1)
	assert.NilError(t, err)

	// Then a second endpoint
	ep2 := &Endpoint{network: nw, id: "testEndpoint2"}
	err = c.storeEndpoint(context.Background(), ep2)
	assert.NilError(t, err)

	// Check that we can find both endpoints, and that the returned values are
	// not copies of the original ones.
	found := c.findEndpoints(filterEndpointByNetworkId("testNetwork"))
	slices.SortFunc(found, func(a, b *Endpoint) int { return strings.Compare(a.id, b.id) })
	assert.Equal(t, len(found), 2)
	assert.Check(t, is.Equal(found[0], ep1), "got: %s; expected: %s", found[0].id, ep1.id)
	assert.Check(t, is.Equal(found[1], ep2), "got: %s; expected: %s", found[1].id, ep1.id)

	// Delete the first endpoint
	err = c.deleteStoredEndpoint(ep1)
	assert.NilError(t, err)

	// Check that we can only find the second endpoint
	found = c.findEndpoints(filterEndpointByNetworkId("testNetwork"))
	assert.Equal(t, len(found), 1)
	assert.Check(t, is.Equal(found[0], ep2), "got: %s; expected: %s", found[0].id, ep2.id)

	// Store the second endpoint again
	err = c.storeEndpoint(context.Background(), ep2)
	assert.NilError(t, err)
}