ci: skip running TestParallelConnections locally - #71
Conversation
| if !IsRunningSystemd() { | ||
| t.Skip("Test requires systemd.") | ||
| } | ||
| if _, ok := os.LookupEnv("CGROUPS_ALLOW_UNSAFE_TESTS"); !ok { |
There was a problem hiding this comment.
nit: strconv.ParseBool should be used to allow negative boolean values
Alternatively you can define a -test.*=<BOOL> flag
There was a problem hiding this comment.
-
Can do
strconv.ParseBoolhere, yet I was just mimicking what we have in runc (variable presense/absense is a flag, not its value). -
A CLI flag is more cumbersome to use in complex scenarios, as you have to specify it every time you call
go testor the test binary. With an environment variable, you only have to set it once for every test environment.
There was a problem hiding this comment.
Also, strconv.ParseBool fails when the value is unset (os.Getenv() returns an empty string, so the proper handling is becoming a burden, whereas the current solution merely relies on env var being present/absent.
I'd rather keep this as is (simple), unless there's a compelling reason not to.
| t.Skip("Test requires systemd.") | ||
| } | ||
| if _, ok := os.LookupEnv("CGROUPS_ALLOW_UNSAFE_TESTS"); !ok { | ||
| t.Skip("unsafe test (requires CGROUPS_ALLOW_UNSAFE_TESTS env var set)") |
There was a problem hiding this comment.
Maybe add a comment line to explain how it is unsafe
I nuked my GNOME session twice today by running go test ./... here.
Apparently what happens is too many user's connections result in this:
dbus-broker-launch: ERROR sockopt_get_peerpidfd: Too many open files
peer_new_with_fd @ ../src/bus/peer.c +290
listener_dispatch @ ../src/bus/listener.c +54
dbus-broker-launch: Caught SIGCHLD of broker.
and then everything dies.
Let's not run this test unless CGROUPS_ALLOW_UNSAFE_TESTS is set (and
set this in GHA CI).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2 participants
I nuked my GNOME session twice today by running
go test ./...on this repo.Apparently what happens is too many user's connections created by
TestParallelConnectionresult in this:Apparently dbus-broker sees EMFILE and dies, taking
the whole desktop session down with it. 💣
Let's not run this test unless
CGROUPS_ALLOW_UNSAFE_TESTSis set (and set this in GHA CI).