Test fixes

[kolyshkin]

1. mountinfo: TestMountedBy: require root

   This test needs root as its setup performs some mounts.
   It used to require root (indirectly via requireOpenat2),
   but after commit 098ed00 (PR mountinfo: TestMountedBy: skip mountedByOpenat2() check if not supported #67)
   this is no longer the case, and thus if passwordless sudo
   is not available, the test fails:

        [kir@kir-rhat sys]$ make test
        sudo: a password is required
        for p in mountinfo mount signal symlink; do \
                (cd $p && go test  -v .); \
        done
        === RUN   TestMountedBy
            mounted_linux_test.go:221: setup failed: mount /tmp/TestMountedBy1220310657/tmpfs-mount: operation not permitted
        --- FAIL: TestMountedBy (0.00s)
   

   Add an explicit root check to fix this.

   Fixes: 098ed00

2. mount: also test against local mountinfo source code

   mount depends on mountinfo, but defaults to the release specified in go.mod.

   This change adds a new test to also run against the local code so that we
   can catch regressions / breaking changes.

   (This is a carry of mount: also test against local mountinfo source code #68, with some minor changes shown in mount: also test against local mountinfo source code #68 (comment))

Closes: #68

[thaJeztah]

Thanks! Left a comment about adding go mod tidy (for the local case)

[thaJeztah]

So, perhaps we don't need a go mod download, as go test will implicitly do so, but we likely do need a go mod tidy, in which case we can drop the cp (I'll write more details below).

Suggested change

	cp mount/go.sum mount/go-local.sum
	cd mount && go test -modfile=go-local.mod $(RUN_VIA_SUDO) -v .
	cd mount && go mod tidy -modfile=go-local.mod && go test -modfile=go-local.mod $(RUN_VIA_SUDO) -v .

[thaJeztah]

If we don't do a go mod tidy, then the go-local.mod and go-local.sum don't take (indirect) dependency changes from the local copy of mountinfo into account.

For example, if we would update mountinfo to use the latest golang.org/x/sys:

cd mountinfo/
go get golang.org/x/sys@latest
# go: downloading golang.org/x/sys v0.0.0-20211025201205-69cdffdb9359
# go get: upgraded golang.org/x/sys v0.0.0-20200909081042-eff7692f9009 => v0.0.0-20211025201205-69cdffdb9359

go mod tidy
cd ../

Then, try make test-local without go mod tidy;

make test-local
/bin/sh: 1: sudo: not found
echo 'replace github.com/moby/sys/mountinfo => ../mountinfo' | cat mount/go.mod - > mount/go-local.mod
cp mount/go.sum mount/go-local.sum
cd mount && go test -modfile=go-local.mod  -v .
go: github.com/moby/sys/mountinfo@v0.4.1 requires
	golang.org/x/sys@v0.0.0-20211025201205-69cdffdb9359: missing go.sum entry; to add it:
	go mod download golang.org/x/sys
go: github.com/moby/sys/mountinfo@v0.4.1 requires
	golang.org/x/sys@v0.0.0-20211025201205-69cdffdb9359: missing go.sum entry; to add it:
	go mod download golang.org/x/sys
make: *** [Makefile:29: test-local] Error 1

And here's after adding a go mod tidy -modfile=go-local.mod (also commented-out the $(RM) to check diff afterwards;

git diff -- Makefile

diff --git a/Makefile b/Makefile
index b4a196e..52f7d1e 100644
--- a/Makefile
+++ b/Makefile
@@ -26,8 +26,8 @@ test: test-local
 test-local:
 	echo 'replace github.com/moby/sys/mountinfo => ../mountinfo' | cat mount/go.mod - > mount/go-local.mod
 	cp mount/go.sum mount/go-local.sum
-	cd mount && go test -modfile=go-local.mod $(RUN_VIA_SUDO) -v .
-	$(RM) mount/go-local.*
+	cd mount && go mod tidy -modfile=go-local.mod && go test -modfile=go-local.mod $(RUN_VIA_SUDO) -v .
+#	$(RM) mount/go-local.*

 .PHONY: lint
 lint: $(BINDIR)/golangci-lint

After that, run make test-local again, and check go-local.mod and go-local.sum for differences:

git diff --no-index mount/go.mod mount/go-local.mod
git diff --no-index mount/go.sum mount/go-local.sum

diff --git a/mount/go.mod b/mount/go-local.mod
index b08f010..b64dd45 100644
--- a/mount/go.mod
+++ b/mount/go-local.mod
@@ -4,5 +4,7 @@ go 1.14

 require (
 	github.com/moby/sys/mountinfo v0.4.1
-	golang.org/x/sys v0.0.0-20200922070232-aee5d888a860
+	golang.org/x/sys v0.0.0-20211025201205-69cdffdb9359
 )
+
+replace github.com/moby/sys/mountinfo => ../mountinfo

diff --git a/mount/go.sum b/mount/go-local.sum
index a2a62b9..c257a6a 100644
--- a/mount/go.sum
+++ b/mount/go-local.sum
@@ -1,5 +1,2 @@
-github.com/moby/sys/mountinfo v0.4.1 h1:1O+1cHA1aujwEwwVMa2Xm2l+gIpUHyd3+D+d7LZh1kM=
-github.com/moby/sys/mountinfo v0.4.1/go.mod h1:rEr8tzG/lsIZHBtN/JjGG+LMYx9eXgW2JI+6q0qou+A=
-golang.org/x/sys v0.0.0-20200909081042-eff7692f9009/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
-golang.org/x/sys v0.0.0-20200922070232-aee5d888a860 h1:YEu4SMq7D0cmT7CBbXfcH0NZeuChAXwsHe/9XueUO6o=
-golang.org/x/sys v0.0.0-20200922070232-aee5d888a860/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.0.0-20211025201205-69cdffdb9359 h1:2B5p2L5IfGiD7+b9BOoRMC6DgObAVZV+Fsp050NqXik=
+golang.org/x/sys v0.0.0-20211025201205-69cdffdb9359/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=

[kolyshkin]

OK, that happens because mount and mountinfo requires different version of golang.org/x/sys in your local copy.

It's the same for the master branch

[kir@kir-rhat sys]$ grep x/sys mount*/go.mod
mount/go.mod:	golang.org/x/sys v0.0.0-20200922070232-aee5d888a860
mountinfo/go.mod:require golang.org/x/sys v0.0.0-20200909081042-eff7692f9009

Although for some reason I did not see the same behavior as you saw.

[kolyshkin]

I propose we add a check that all modules (or at least mount and mountinfo, since one requires the other) use the same version of x/sys, WDYT @thaJeztah? Once done, this is no longer a problem.

[kolyshkin]

I propose we add a check that all modules (or at least mount and mountinfo, since one requires the other) use the same version of x/sys, WDYT @thaJeztah? Once done, this is no longer a problem.

OK, that's too complicated to do in the current version. Makes sense to do for a future version but separately.

Going back to go mod download.

[kolyshkin]

Although for some reason I did not see the same behavior as you saw.

GHA CI does not show it either. I am mildly confused 😕

[thaJeztah]

OK, that happens because mount and mountinfo requires different version of golang.org/x/sys in your local copy.

Yes, I intentionally updated the version in my example to illustrate how testing against the "local" module versus "the version in go.mod" could affect the outcome; if mountinfo updated its dependencies, it would force mount to also update its version, which could have side effects, so having that as a test in CI would help us get early warning that the mountinfo in master would break mount if we would update the version of mountinfo there.

Although for some reason I did not see the same behavior as you saw.

Hmm... did you first update the golang.org/x/sys in mounting?

cd mountinfo/
go get golang.org/x/sys@latest
go mod tidy

I propose we add a check that all modules (or at least mount and mountinfo, since one requires the other)

Yeah, perhaps we don't need it, as long as we test that "once updating, this is what happens". I assume we want those updates (ideally) in separate commits, so not sure it's worth doing.

[kolyshkin]

Updated with go mod tidy instead of cp, PTAL @thaJeztah.

[thaJeztah]

CI is failing; 😅

go mod test: unknown command

(left a comment inline)

[thaJeztah]

LGTM, thanks!