Skip to content

ci: skip running TestParallelConnections locally #71

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kolyshkin wants to merge 1 commit into
base: main
Choose a base branch
from
+6 −2
Open

ci: skip running TestParallelConnections locally #71

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ jobs:
- uses: actions/setup-go@v7
with:
go-version: ${{ matrix.go-version }}
- run: go test -timeout 3m ${{ matrix.race }} -v ./...
- run: CGROUPS_ALLOW_UNSAFE_TESTS=true go test -timeout 3m ${{ matrix.race }} -v ./...

cgroup-v1:
name: "cgroup v1 (AlmaLinux 8)"
Expand Down Expand Up @@ -57,7 +57,7 @@ jobs:
lima sudo dnf install -y golang

- name: "Run unit tests"
run: LIMA_WORKDIR=/tmp/cgroups lima sudo GOTOOLCHAIN=auto go test -v ./...
run: LIMA_WORKDIR=/tmp/cgroups lima sudo GOTOOLCHAIN=auto CGROUPS_ALLOW_UNSAFE_TESTS=true go test -v ./...

all-done:
needs:
Expand Down
4 changes: 4 additions & 0 deletions systemd/dbus_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ func TestParallelConnection(t *testing.T) {
if !IsRunningSystemd() {
t.Skip("Test requires systemd.")
}
if _, ok := os.LookupEnv("CGROUPS_ALLOW_UNSAFE_TESTS"); !ok {

@AkihiroSuda AkihiroSuda Aug 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: strconv.ParseBool should be used to allow negative boolean values

Alternatively you can define a -test.*=<BOOL> flag

@kolyshkin kolyshkin Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Can do strconv.ParseBool here, yet I was just mimicking what we have in runc (variable presense/absense is a flag, not its value).

  2. A CLI flag is more cumbersome to use in complex scenarios, as you have to specify it every time you call go test or the test binary. With an environment variable, you only have to set it once for every test environment.

@kolyshkin kolyshkin Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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("skipping unsafe test (can kill your desktop session); " +
"set CGROUPS_ALLOW_UNSAFE_TESTS=true to enable")
}
var dms []*dbusConnManager
for range 600 {
dms = append(dms, newDbusConnManager(os.Geteuid() != 0))
Expand Down