Skip to content
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

Semiperiodic cleanup of obsolete Skip()s #12349

Merged
merged 1 commit into from
Nov 22, 2021
Merged

Semiperiodic cleanup of obsolete Skip()s #12349

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
5 changes: 4 additions & 1 deletion test/e2e/common_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ func (p *PodmanTestIntegration) createArtifact(image string) {
}
dest := strings.Split(image, "/")
destName := fmt.Sprintf("/tmp/%s.tar", strings.Replace(strings.Join(strings.Split(dest[len(dest)-1], "/"), ""), ":", "-", -1))
fmt.Printf("Caching %s at %s...", image, destName)
fmt.Printf("Caching %s at %s...\n", image, destName)
Copy link
Member Author

Choose a reason for hiding this comment

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

This is immediately followed by a verbose podman pull, which spits out Running: etc on the same line. My logformatter does not see this command and does not perform its helpful highlights.

if _, err := os.Stat(destName); os.IsNotExist(err) {
pull := p.PodmanNoCache([]string{"pull", image})
pull.Wait(440)
Expand Down Expand Up @@ -466,6 +466,9 @@ func (p *PodmanTestIntegration) BuildImageWithLabel(dockerfile, imageName string
// PodmanPID execs podman and returns its PID
func (p *PodmanTestIntegration) PodmanPID(args []string) (*PodmanSessionIntegration, int) {
podmanOptions := p.MakeOptions(args, false, false)
if p.RemoteTest {
podmanOptions = append([]string{"--remote", "--url", p.RemoteSocket}, podmanOptions...)
}
fmt.Printf("Running: %s %s\n", p.PodmanBinary, strings.Join(podmanOptions, " "))
command := exec.Command(p.PodmanBinary, podmanOptions...)
session, err := Start(command, GinkgoWriter, GinkgoWriter)
Expand Down
3 changes: 2 additions & 1 deletion test/e2e/import_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ var _ = Describe("Podman import", func() {
)

BeforeEach(func() {
SkipIfRemote("FIXME: These look like it is supposed to work in remote")
tempdir, err = CreateTempDirInTempDir()
if err != nil {
os.Exit(1)
Expand Down Expand Up @@ -156,6 +155,8 @@ var _ = Describe("Podman import", func() {
})

It("podman import with signature", func() {
SkipIfRemote("FIXME: remote ignores --signature-policy, #12357")

outfile := filepath.Join(podmanTest.TempDir, "container.tar")
_, ec, cid := podmanTest.RunLsContainer("")
Expect(ec).To(Equal(0))
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/logs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ var _ = Describe("Podman logs", func() {

It("two containers showing short container IDs: "+log, func() {
skipIfJournaldInContainer()
SkipIfRemote("FIXME: podman-remote logs does not support showing two containers at the same time")
SkipIfRemote("podman-remote logs does not support showing two containers at the same time")
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed the FIXME, because this does not look like it's going to be fixed


log1 := podmanTest.Podman([]string{"run", "--log-driver", log, "-dt", ALPINE, "sh", "-c", "echo podman; echo podman; echo podman"})
log1.WaitWithDefaultTimeout()
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/push_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ var _ = Describe("Podman push", func() {
})

It("podman push to local registry with authorization", func() {
SkipIfRootless("FIXME: Creating content in certs.d we use directories in homedir")
SkipIfRootless("volume-mounting a certs.d file N/A over remote")
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed the FIXME, because this does not look like it's going to be fixed

if podmanTest.Host.Arch == "ppc64le" {
Skip("No registry image for ppc64le")
}
Expand Down
16 changes: 8 additions & 8 deletions test/e2e/run_cgroup_parent_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,21 +48,22 @@ var _ = Describe("Podman run with --cgroup-parent", func() {
run := podmanTest.Podman([]string{"run", "--cgroupns=host", "--cgroup-parent", cgroup, fedoraMinimal, "cat", "/proc/self/cgroup"})
run.WaitWithDefaultTimeout()
Expect(run).Should(Exit(0))
ok, _ := run.GrepString(cgroup)
Expect(ok).To(BeTrue())
Expect(run.OutputToString()).To(ContainSubstring(cgroup))
Copy link
Member Author

Choose a reason for hiding this comment

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

I really can't begin to express how anti-helpful it is to see "zero is not one" in ginkgo error logs.

})

Specify("no --cgroup-parent", func() {
SkipIfRootless("FIXME This seems to be broken in rootless mode")
cgroup := "/libpod_parent"
if !Containerized() && podmanTest.CgroupManager != "cgroupfs" {
cgroup = "/machine.slice"
if isRootless() {
cgroup = "/user.slice"
} else {
cgroup = "/machine.slice"
}
}
run := podmanTest.Podman([]string{"run", "--cgroupns=host", fedoraMinimal, "cat", "/proc/self/cgroup"})
run.WaitWithDefaultTimeout()
Expect(run).Should(Exit(0))
ok, _ := run.GrepString(cgroup)
Expect(ok).To(BeTrue())
Expect(run.OutputToString()).To(ContainSubstring(cgroup))
})

Specify("always honor --cgroup-parent", func() {
Expand Down Expand Up @@ -114,7 +115,6 @@ var _ = Describe("Podman run with --cgroup-parent", func() {
run := podmanTest.Podman([]string{"run", "--cgroupns=host", "--cgroup-parent", cgroup, fedoraMinimal, "cat", "/proc/1/cgroup"})
run.WaitWithDefaultTimeout()
Expect(run).Should(Exit(0))
ok, _ := run.GrepString(cgroup)
Expect(ok).To(BeTrue())
Expect(run.OutputToString()).To(ContainSubstring(cgroup))
})
})
2 changes: 1 addition & 1 deletion test/e2e/run_cleanup_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ var _ = Describe("Podman run exit", func() {

It("podman run -d mount cleanup test", func() {
SkipIfRemote("podman-remote does not support mount")
SkipIfRootless("FIXME podman mount requires podman unshare first")
SkipIfRootless("TODO rootless podman mount requires podman unshare first")
Copy link
Member Author

Choose a reason for hiding this comment

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

Changed FIXME to TODO. The difference, as I see it, is that TODO is "that would take effort. Maybe some day we'll enhance the test". Whereas FIXME is "this is broken". (And my find-obsolete-skips script flags FIXME but not TODO)


result := podmanTest.Podman([]string{"run", "-dt", ALPINE, "top"})
result.WaitWithDefaultTimeout()
Expand Down
1 change: 0 additions & 1 deletion test/e2e/run_privileged_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,6 @@ var _ = Describe("Podman privileged container tests", func() {
})

It("podman privileged should inherit host devices", func() {
SkipIfRootless("FIXME: This seems to be broken for rootless mode, /dev/ is close to the same")
session := podmanTest.Podman([]string{"run", "--privileged", ALPINE, "ls", "-l", "/dev"})
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(0))
Expand Down
2 changes: 0 additions & 2 deletions test/e2e/run_signal_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ var _ = Describe("Podman run with --sig-proxy", func() {
})

Specify("signals are forwarded to container using sig-proxy", func() {
SkipIfRemote("FIXME: This looks like it is supposed to work in remote")
Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed by the PodmanPID change at top

if podmanTest.Host.Arch == "ppc64le" {
Skip("Doesn't work on ppc64le")
}
Expand Down Expand Up @@ -111,7 +110,6 @@ var _ = Describe("Podman run with --sig-proxy", func() {
})

Specify("signals are not forwarded to container with sig-proxy false", func() {
SkipIfRemote("FIXME: This looks like it is supposed to work in remote")
signal := syscall.SIGFPE
if rootless.IsRootless() {
podmanTest.RestoreArtifact(fedoraMinimal)
Expand Down
8 changes: 4 additions & 4 deletions test/e2e/run_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,7 @@ USER bin`, BB)
})

It("podman run device-read-bps test", func() {
SkipIfRootless("FIXME: Missing /sys/fs/cgroup/user.slice/user-14467.slice/[email protected]/cgroup.subtree_control")
SkipIfRootless("FIXME: requested cgroup controller `io` is not available")
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the new error message as of this writing.

SkipIfRootlessCgroupsV1("Setting device-read-bps not supported on cgroupv1 for rootless users")

var session *PodmanSessionIntegration
Expand All @@ -700,7 +700,7 @@ USER bin`, BB)
})

It("podman run device-write-bps test", func() {
SkipIfRootless("FIXME /sys/fs/cgroup/user.slice/user-14467.slice/[email protected]/cgroup.subtree_control does not exist")
SkipIfRootless("FIXME: requested cgroup controller `io` is not available")
SkipIfRootlessCgroupsV1("Setting device-write-bps not supported on cgroupv1 for rootless users")

var session *PodmanSessionIntegration
Expand All @@ -718,7 +718,7 @@ USER bin`, BB)
})

It("podman run device-read-iops test", func() {
SkipIfRootless("FIXME /sys/fs/cgroup/user.slice/user-14467.slice/[email protected]/cgroup.subtree_control does not exist")
SkipIfRootless("FIXME: requested cgroup controller `io` is not available")
SkipIfRootlessCgroupsV1("Setting device-read-iops not supported on cgroupv1 for rootless users")
var session *PodmanSessionIntegration

Expand All @@ -736,7 +736,7 @@ USER bin`, BB)
})

It("podman run device-write-iops test", func() {
SkipIfRootless("FIXME /sys/fs/cgroup/user.slice/user-14467.slice/[email protected]/cgroup.subtree_control does not exist")
SkipIfRootless("FIXME: requested cgroup controller `io` is not available")
SkipIfRootlessCgroupsV1("Setting device-write-iops not supported on cgroupv1 for rootless users")
var session *PodmanSessionIntegration

Expand Down
4 changes: 2 additions & 2 deletions test/e2e/run_volume_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,8 +282,8 @@ var _ = Describe("Podman run with volumes", func() {
})

It("podman run with tmpfs named volume mounts and unmounts", func() {
SkipIfRootless("FIXME: rootless podman mount requires you to be in a user namespace")
SkipIfRemote("podman-remote does not support --volumes this test could be simplified to be tested on Remote.")
SkipIfRootless("rootless podman mount requires you to be in a user namespace")
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed the FIXME. Maybe this should be a TODO instead?

SkipIfRemote("podman-remote does not support --volumes. This test could be simplified to be tested on Remote.")
volName := "testvol"
mkVolume := podmanTest.Podman([]string{"volume", "create", "--opt", "type=tmpfs", "--opt", "device=tmpfs", "--opt", "o=nodev", "testvol"})
mkVolume.WaitWithDefaultTimeout()
Expand Down
2 changes: 1 addition & 1 deletion test/system/030-run.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ echo $rand | 0 | $rand
}

@test "podman run docker-archive" {
skip_if_remote "podman-remote does not support docker-archive (#7116)"
skip_if_remote "podman-remote does not support docker-archive"
Copy link
Member Author

Choose a reason for hiding this comment

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

This will never be fixed. Remove the reference.


# Create an image that, when run, outputs a random magic string
expect=$(random_string 20)
Expand Down
5 changes: 0 additions & 5 deletions test/system/400-unprivileged-access.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,6 @@ EOF

# #6957 - mask out /proc/acpi, /sys/dev, and other sensitive system files
@test "sensitive mount points are masked without --privileged" {
# Weird error, maybe a flake?
# can only attach to created or running containers: container state improper
# https://github.com/containers/podman/pull/7111#issuecomment-666858715
skip_if_remote "FIXME: Weird flake"

# FIXME: this should match the list in pkg/specgen/generate/config_linux.go
local -a mps=(
/proc/acpi
Expand Down
1 change: 1 addition & 0 deletions test/utils/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ func (p *PodmanTest) WaitForContainer() bool {
}
time.Sleep(1 * time.Second)
}
fmt.Printf("WaitForContainer(): timed out\n")
Copy link
Member Author

Choose a reason for hiding this comment

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

This would've been really helpful when I was debugging the PodmanPID bug above.

return false
}

Expand Down