On 2024-07-24 10:42, Jochen Sprickerhof wrote:
* Reinhard Tartler <siret...@tauware.de> [2024-07-24 09:36]:
Looking closer at the tests, it appears that they are iterating over
all installed network interfaces, and try to query them for
statistics. In that 'unshare -nr' context, they still find a loopback
interface, but commands to query fail with "operation not supported".
Clearly, those tests don't make sense in such an environment.
Jochen, what's the best way to detect this situation in a test, so
that it can decide to do a "t.Skip(...)" instead of failing? I'm
looking for a way to avoid having to disable the tests altogether to
not loose test coverage on machines that don't run in unshare mode.
Looking at:
https://sources.debian.org/src/golang-github-safchain-ethtool/0.4.1-1/ethtool_test.go/#L45
The skip condition could depend on the return value of net.Interfaces()
but I guess it would be basically the same as the fail condition.
Even in an `unshare -nr` environment the net.Interfaces() correctly
identifies the `lo` interface without issues. It is the subsequent calls
that fail.
I think the correct way would be to mock the net.Interfaces() return
value such that the same test data is used, independent of the host
system.
Debating about the "correct" religion/philosophy of what is a unit test
and what isn't comes at the cost of breaking existing tests that have
value as it is. And by extension, this causes packages to FTBFS like in
this case, distracting already scarce Developer time from more important
issues.
on a side note, I'd like to express concerns with having the buildds
enable unshare mode becaues it evidently reduces test coverage, as
shown in this context.
I disagree with this as unit tests can (and should) always mock the
system state.
I guess we have to agree to disagree. In the mean time, I've uploaded a
change that disables that test. I'm looking forward to integrate your
patch that "properly" "mocks the system state" so that we can restore
the test coverage, which removal was triggered by this (upcoming?)
change to the buildds to use schroot's "unshare" mode.
-rt
