From d9729af78e0f52c7b771adf9b3d67486904f9ebf Mon Sep 17 00:00:00 2001 From: Tiexin Guo Date: Sun, 22 Sep 2024 18:49:52 +0800 Subject: [PATCH 1/4] test: make root tests easier to run --- .github/workflows/tests.yml | 11 +---------- internals/cli/cmd_add_test.go | 3 +++ internals/osutil/io_test.go | 3 +++ internals/osutil/stat_test.go | 7 +++++++ 4 files changed, 14 insertions(+), 10 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..9233dd7cd 100644 --- a/internals/cli/cmd_add_test.go +++ b/internals/cli/cmd_add_test.go @@ -100,6 +100,9 @@ services: } else if path == triggerLayerPath { c.Assert(err, check.ErrorMatches, "triggered") } else if path == unreadableLayerPath { + if os.Getuid() == 0 { + c.Skip("requires running as non-root users") + } c.Assert(os.IsPermission(err), check.Equals, true) } } diff --git a/internals/osutil/io_test.go b/internals/osutil/io_test.go index c4040e8bb..b6c00d2a4 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 users") + } err := osutil.AtomicWriteFile(p, []byte("hi"), 0600, 0) c.Assert(err, NotNil) } diff --git a/internals/osutil/stat_test.go b/internals/osutil/stat_test.go index 5ff2e5a46..b90dad574 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 users") + } + 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 users") + } p := makeTestPath(c, "foo/bar", 0) c.Assert(os.Chmod(filepath.Dir(p), 0), IsNil) defer os.Chmod(filepath.Dir(p), 0755) From 8d2dd3b403094c65f8b87f55a7abb6390002ab27 Mon Sep 17 00:00:00 2001 From: Tiexin Guo Date: Sun, 22 Sep 2024 18:58:19 +0800 Subject: [PATCH 2/4] chore: remove unnecessary comments --- internals/daemon/api_exec_test.go | 2 -- internals/daemon/api_files_test.go | 2 -- internals/osutil/mkdir_test.go | 1 - internals/overlord/servstate/manager_test.go | 1 - 4 files changed, 6 deletions(-) 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/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/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) From 79bb611c22e39407f5259e106803f46f26b22bf4 Mon Sep 17 00:00:00 2001 From: Tiexin Guo Date: Mon, 23 Sep 2024 09:07:16 +0800 Subject: [PATCH 3/4] chore: refactor after review --- internals/cli/cmd_add_test.go | 5 ++--- internals/osutil/io_test.go | 2 +- internals/osutil/stat_test.go | 4 ++-- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/internals/cli/cmd_add_test.go b/internals/cli/cmd_add_test.go index 9233dd7cd..83db69da7 100644 --- a/internals/cli/cmd_add_test.go +++ b/internals/cli/cmd_add_test.go @@ -100,10 +100,9 @@ services: } else if path == triggerLayerPath { c.Assert(err, check.ErrorMatches, "triggered") } else if path == unreadableLayerPath { - if os.Getuid() == 0 { - c.Skip("requires running as non-root users") + if os.Getuid() != 0 { + c.Assert(os.IsPermission(err), check.Equals, true) } - c.Assert(os.IsPermission(err), check.Equals, true) } } diff --git a/internals/osutil/io_test.go b/internals/osutil/io_test.go index b6c00d2a4..686a9eb62 100644 --- a/internals/osutil/io_test.go +++ b/internals/osutil/io_test.go @@ -85,7 +85,7 @@ func (ts *AtomicWriteTestSuite) TestAtomicWriteFileSymlinkNoFollow(c *C) { defer os.Chmod(rodir, 0700) if os.Getuid() == 0 { - c.Skip("requires running as non-root users") + 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/stat_test.go b/internals/osutil/stat_test.go index b90dad574..51393e060 100644 --- a/internals/osutil/stat_test.go +++ b/internals/osutil/stat_test.go @@ -110,7 +110,7 @@ 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 users") + c.Skip("requires running as non-root user") } for _, t := range []struct { @@ -185,7 +185,7 @@ func (s *StatTestSuite) TestExistsIsDir(c *C) { } if os.Getuid() == 0 { - c.Skip("requires running as non-root users") + c.Skip("requires running as non-root user") } p := makeTestPath(c, "foo/bar", 0) c.Assert(os.Chmod(filepath.Dir(p), 0), IsNil) From d1243447ad00f528b0902f581d01ef114fda4d03 Mon Sep 17 00:00:00 2001 From: Tiexin Guo Date: Mon, 23 Sep 2024 10:29:27 +0800 Subject: [PATCH 4/4] chore: fix test --- internals/cli/cmd_add_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internals/cli/cmd_add_test.go b/internals/cli/cmd_add_test.go index 83db69da7..a9990c581 100644 --- a/internals/cli/cmd_add_test.go +++ b/internals/cli/cmd_add_test.go @@ -109,5 +109,6 @@ services: args = append(args, "extra", "arguments", "invalid") _, err = cli.ParserForTest().ParseArgs(args) c.Assert(err, check.Equals, cli.ErrExtraArgs) + s.ResetStdStreams() } }