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](#500 (comment)), 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.
canonical · Sep 23, 2024 · 3fce9ec · 3fce9ec
1 parent 183eadf
commit 3fce9ec
Show file tree

Hide file tree

Showing 8 changed files with 15 additions and 17 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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)