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.

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

ci-fix: healthcheck tests should use .Should() instead of .To(). #11077

Merged
merged 1 commit into from
Jul 29, 2021

Conversation

flouthoc
Copy link
Collaborator

Hi Team,

Another PR (#11075) is blocked because of this. Created a separate PR so this can be merged quickly and no new PR gets stuck because of this. I am not really sure please feel free to close this PR if its not needed.

CI error

# Check if the files can be loaded by the shell
. completions/bash/podman
if [ -x /bin/zsh ]; then /bin/zsh completions/zsh/_podman; fi
if [ -x /bin/fish ]; then /bin/fish completions/fish/podman.fish; fi
CGO_ENABLED=1 \
	go build \
	-mod=vendor  \
	-gcflags 'all=-trimpath=/var/tmp/go/src/github.com/containers/podman' \
	-asmflags 'all=-trimpath=/var/tmp/go/src/github.com/containers/podman' \
	-ldflags '-X github.com/containers/podman/v3/libpod/define.gitCommit=6af6f197c3da59899bff1a7d1047e081ee7a382c -X github.com/containers/podman/v3/libpod/define.buildInfo=1627568292 -X github.com/containers/podman/v3/libpod/config._installPrefix=/usr/local -X github.com/containers/podman/v3/libpod/config._etcDir=/usr/local/etc ' \
	-tags "   selinux systemd exclude_graphdriver_devicemapper seccomp" \
	-o bin/podman ./cmd/podman
hack/man-page-checker
hack/xref-helpmsgs-manpages
hack/swagger-check
contrib/cirrus/pr-should-include-tests
test/e2e/healthcheck_run_test.go:		Expect(session.ExitCode()).To(Equal(0))
test/e2e/healthcheck_run_test.go:		Expect(hc.ExitCode()).To(Equal(1))
^^^ Unhelpful use of Expect(ExitCode())
   Please use '.Should(Exit(...))' pattern instead.
   If that's not possible, please add an annotation (description) to your assertion:
        Expect(...).To(..., "Friendly explanation of this check")
make: *** [Makefile:610: tests-expect-exit] Error 1

Exit status: 2

Thanks

Copy link
Member

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

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

LGTM

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 29, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: flouthoc, Luap99

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 29, 2021
@edsantiago
Copy link
Member

edsantiago commented Jul 29, 2021

LGTM. I don't understand how this happened; that commit was merged June 4, much before my ExitCode PR. Thank you for taking care of this.

UPDATE: no, I see now, it was #11048, it must've been based on an old main. This makes sense now.

@Luap99
Copy link
Member

Luap99 commented Jul 29, 2021

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 29, 2021
@openshift-merge-robot openshift-merge-robot merged commit f17b810 into containers:main Jul 29, 2021
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 22, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants