Skip to content

Commit

Permalink
ipsec: Remove unit tests for xfrm_collector
Browse files Browse the repository at this point in the history
Remove the unit tests for xfrm_collector. They are simply not worth it.
All xfrm_collector does is collect statistics via helper functions and
feed them into Prometheus functions. The helper functions are covered by
unit tests. Prometheus functions are covered by unit tests. We're really
just passing the information from one to the other.

But to cover this trivial logic in unit tests, we end up copying lots of
information across the code and tests. We also need to pass the helper
functions as a function pointer to xfrm_collector. Every new metric is
going to need a new function pointer.

I believe this is a waste of engineering time and very unlikely to catch
mistakes. It doesn't even test the helper functions since we mock them
out. So it's really unclear what we're trying to cover here. Removing it
makes it easier to add new metrics in followup commits.

Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
  • Loading branch information
pchaigno authored and joestringer committed Oct 19, 2023
1 parent 2218611 commit 092d661
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 178 deletions.
12 changes: 1 addition & 11 deletions pkg/datapath/linux/ipsec/xfrm_collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,22 +37,12 @@ const (
)

type xfrmCollector struct {
xfrmStatFunc func() (procfs.XfrmStat, error)

// Inbound errors
xfrmErrorDesc *prometheus.Desc
}

// NewXFRMCollector returns a new prometheus.Collector for /proc/net/xfrm_stat
// https://www.kernel.org/doc/Documentation/networking/xfrm_proc.txt
func NewXFRMCollector() prometheus.Collector {
return newXFRMCollector(procfs.NewXfrmStat)
}

func newXFRMCollector(statFn func() (procfs.XfrmStat, error)) prometheus.Collector {
return &xfrmCollector{
xfrmStatFunc: statFn,

xfrmErrorDesc: prometheus.NewDesc(
prometheus.BuildFQName(metrics.Namespace, subsystem, "xfrm_error"),
"Total number of xfrm errors",
Expand All @@ -66,7 +56,7 @@ func (x *xfrmCollector) Describe(ch chan<- *prometheus.Desc) {
}

func (x *xfrmCollector) Collect(ch chan<- prometheus.Metric) {
stats, err := x.xfrmStatFunc()
stats, err := procfs.NewXfrmStat()
if err != nil {
log.WithError(err).Error("Error while getting xfrm stats")
return
Expand Down
167 changes: 0 additions & 167 deletions pkg/datapath/linux/ipsec/xfrm_collector_test.go

This file was deleted.

0 comments on commit 092d661

Please sign in to comment.