From 3fce9ec0fc773abbff3bba9818a15bb237bbfb7d Mon Sep 17 00:00:00 2001 From: Tiexin Guo Date: Mon, 23 Sep 2024 12:59:46 +0800 Subject: [PATCH] test: make root tests easier to run (#501) Background: Previously, when adding a new root test, we had to modify the GitHub Actions workflow to include the newly added test, which is some extra operational overhead. Per discussion [here](https://github.com/canonical/pebble/pull/500#issuecomment-2362290802), we agreed to run all tests with root and skip those tests that would fail with root (checking permissions and stuff), and this PR skips tests that would fail when running with root and made the GitHub Actions workflow for root tests much easier. --- .github/workflows/tests.yml | 11 +---------- internals/cli/cmd_add_test.go | 5 ++++- internals/daemon/api_exec_test.go | 2 -- internals/daemon/api_files_test.go | 2 -- internals/osutil/io_test.go | 3 +++ internals/osutil/mkdir_test.go | 1 - internals/osutil/stat_test.go | 7 +++++++ internals/overlord/servstate/manager_test.go | 1 - 8 files changed, 15 insertions(+), 17 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index bf0e18e90..f0aa5a6a0 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -43,16 +43,7 @@ jobs: - name: Test run: | - go test -c ./internals/daemon - PEBBLE_TEST_USER=runner PEBBLE_TEST_GROUP=runner sudo -E -H ./daemon.test -check.v -check.f ^execSuite\.TestUserGroup$ - PEBBLE_TEST_USER=runner PEBBLE_TEST_GROUP=runner sudo -E -H ./daemon.test -check.v -check.f ^execSuite\.TestUserIDGroupID$ - PEBBLE_TEST_USER=runner PEBBLE_TEST_GROUP=runner sudo -E -H ./daemon.test -check.v -check.f ^filesSuite\.TestWriteUserGroupReal$ - PEBBLE_TEST_USER=runner PEBBLE_TEST_GROUP=runner sudo -E -H ./daemon.test -check.v -check.f ^filesSuite\.TestMakeDirsUserGroupReal$ - go test -c ./internals/osutil - PEBBLE_TEST_USER=runner PEBBLE_TEST_GROUP=runner sudo -E -H ./osutil.test -check.v -check.f ^mkdirSuite\.TestMakeParentsChmodAndChown$ - - go test -c ./internals/overlord/servstate/ - PEBBLE_TEST_USER=runner PEBBLE_TEST_GROUP=runner sudo -E -H ./servstate.test -check.v -check.f ^S.TestUserGroup$ + PEBBLE_TEST_USER=runner PEBBLE_TEST_GROUP=runner sudo -E -H go test ./... format: runs-on: ubuntu-latest diff --git a/internals/cli/cmd_add_test.go b/internals/cli/cmd_add_test.go index e84b02729..a9990c581 100644 --- a/internals/cli/cmd_add_test.go +++ b/internals/cli/cmd_add_test.go @@ -100,12 +100,15 @@ services: } else if path == triggerLayerPath { c.Assert(err, check.ErrorMatches, "triggered") } else if path == unreadableLayerPath { - c.Assert(os.IsPermission(err), check.Equals, true) + if os.Getuid() != 0 { + c.Assert(os.IsPermission(err), check.Equals, true) + } } } args = append(args, "extra", "arguments", "invalid") _, err = cli.ParserForTest().ParseArgs(args) c.Assert(err, check.Equals, cli.ErrExtraArgs) + s.ResetStdStreams() } } diff --git a/internals/daemon/api_exec_test.go b/internals/daemon/api_exec_test.go index 23ede9ae7..37694882b 100644 --- a/internals/daemon/api_exec_test.go +++ b/internals/daemon/api_exec_test.go @@ -258,7 +258,6 @@ func (s *execSuite) TestCurrentUserGroup(c *C) { c.Check(stderr, Equals, "") } -// See .github/workflows/tests.yml for how to run this test as root. func (s *execSuite) TestUserGroup(c *C) { if os.Getuid() != 0 { c.Skip("requires running as root") @@ -286,7 +285,6 @@ func (s *execSuite) TestUserGroup(c *C) { c.Assert(err, ErrorMatches, `.*home directory.*does not exist`) } -// See .github/workflows/tests.yml for how to run this test as root. func (s *execSuite) TestUserIDGroupID(c *C) { if os.Getuid() != 0 { c.Skip("requires running as root") diff --git a/internals/daemon/api_files_test.go b/internals/daemon/api_files_test.go index 79c0f97e7..e3bc792e7 100644 --- a/internals/daemon/api_files_test.go +++ b/internals/daemon/api_files_test.go @@ -531,7 +531,6 @@ func (s *filesSuite) testMakeDirsUserGroup(c *C, uid, gid int, user, group strin return tmpDir } -// See .github/workflows/tests.yml for how to run this test as root. func (s *filesSuite) TestMakeDirsUserGroupReal(c *C) { if os.Getuid() != 0 { c.Skip("requires running as root") @@ -988,7 +987,6 @@ func (s *filesSuite) TestWriteUserGroupMocked(c *C) { c.Check(mkdirCalls[1], Equals, mkdirArgs{tmpDir + "/nested2", 0o755, osutil.MkdirOptions{MakeParents: true, ExistOK: true, Chmod: true, Chown: true, UserID: 56, GroupID: 78}}) } -// See .github/workflows/tests.yml for how to run this test as root. func (s *filesSuite) TestWriteUserGroupReal(c *C) { if os.Getuid() != 0 { c.Skip("requires running as root") diff --git a/internals/osutil/io_test.go b/internals/osutil/io_test.go index c4040e8bb..686a9eb62 100644 --- a/internals/osutil/io_test.go +++ b/internals/osutil/io_test.go @@ -84,6 +84,9 @@ func (ts *AtomicWriteTestSuite) TestAtomicWriteFileSymlinkNoFollow(c *C) { c.Assert(os.Chmod(rodir, 0500), IsNil) defer os.Chmod(rodir, 0700) + if os.Getuid() == 0 { + c.Skip("requires running as non-root user") + } err := osutil.AtomicWriteFile(p, []byte("hi"), 0600, 0) c.Assert(err, NotNil) } diff --git a/internals/osutil/mkdir_test.go b/internals/osutil/mkdir_test.go index 6983e26da..21668e3ff 100644 --- a/internals/osutil/mkdir_test.go +++ b/internals/osutil/mkdir_test.go @@ -208,7 +208,6 @@ func (mkdirSuite) TestMakeParentsAndNoChmod(c *check.C) { c.Assert(info.Mode().Perm(), check.Equals, os.FileMode(0o755)) } -// See .github/workflows/tests.yml for how to run this test as root. func (mkdirSuite) TestMakeParentsChmodAndChown(c *check.C) { if os.Getuid() != 0 { c.Skip("requires running as root") diff --git a/internals/osutil/stat_test.go b/internals/osutil/stat_test.go index 5ff2e5a46..51393e060 100644 --- a/internals/osutil/stat_test.go +++ b/internals/osutil/stat_test.go @@ -109,6 +109,10 @@ func makeTestPathInDir(c *C, dir string, path string, mode os.FileMode) string { } func (s *StatTestSuite) TestIsWritableDir(c *C) { + if os.Getuid() == 0 { + c.Skip("requires running as non-root user") + } + for _, t := range []struct { path string mode os.FileMode @@ -180,6 +184,9 @@ func (s *StatTestSuite) TestExistsIsDir(c *C) { c.Check(err, IsNil, comm) } + if os.Getuid() == 0 { + c.Skip("requires running as non-root user") + } p := makeTestPath(c, "foo/bar", 0) c.Assert(os.Chmod(filepath.Dir(p), 0), IsNil) defer os.Chmod(filepath.Dir(p), 0755) diff --git a/internals/overlord/servstate/manager_test.go b/internals/overlord/servstate/manager_test.go index 391e41628..c097336ca 100644 --- a/internals/overlord/servstate/manager_test.go +++ b/internals/overlord/servstate/manager_test.go @@ -532,7 +532,6 @@ func (s *S) TestUserGroupFails(c *C) { c.Check(gotGid, Equals, uint32(gid)) } -// See .github/workflows/tests.yml for how to run this test as root. func (s *S) TestUserGroup(c *C) { s.newServiceManager(c) s.planAddLayer(c, testPlanLayer)