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

BATS: add ping test, ps filters, multi-option #8535

Merged
merged 1 commit into from
Dec 1, 2020

Conversation

edsantiago
Copy link
Member

  • run test : tweaks to recently-added network-conflict test:

  • images, run, build, exec tests: add multiple-flag
    testing for various flags, confirming as appropriate
    whether options are overridden or accumulated.

  • ps test : add --filter and --sort tests

  • pod test: run 'ping' inside container (confirms that
    container gets PING capability)

Signed-off-by: Ed Santiago [email protected]

 - run test : tweaks to recently-added network-conflict test:
   * remove "-d" in run
   * confirm exact warning text, and also that container
     runs successfully
   * test multiple --net options (regression containers#8057)

 - images, run, build, exec tests: add multiple-flag
   testing for various flags, confirming as appropriate
   whether options are overridden or accumulated.

 - ps test : add --filter and --sort tests

 - pod test: run 'ping' inside container (confirms that
   container gets PING capability)

Signed-off-by: Ed Santiago <[email protected]>
@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 1, 2020
@edsantiago edsantiago changed the title BATS: add ping test BATS: add ping test, ps filters, multi-option Dec 1, 2020
@TomSweeneyRedHat
Copy link
Member

LGTM
and happy green test buttons.

@rhatdan
Copy link
Member

rhatdan commented Dec 1, 2020

/approve
/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 1, 2020
@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: edsantiago, rhatdan

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-merge-robot openshift-merge-robot merged commit b735aa8 into containers:master Dec 1, 2020
@@ -533,8 +537,15 @@ json-file | f
}

@test "podman run with --net=host and --port prints warning" {
run_podman run -d --rm -p 8080 --net=host $IMAGE ls > /dev/null
Copy link
Member Author

Choose a reason for hiding this comment

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

Quick followup on this: it turns out, >/dev/null is not a NOP, it's worse: it causes all output from run_podman to vanish, which is anti-helpful when this test flakes, as it did twice in #8536!

remote f33

remote ubuntu-20

Copy link
Member

Choose a reason for hiding this comment

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

The intent is to remove all non-stderr output. We only want to capture the output of STDERR for this test. I believe you told me this was the best way to accomplish that?

Regarding the flake, I'll bet is the --rm conflicting with the test's own cleanup logic.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, that makes sense (the rm conflict).

Re: BATS and stdout/stderr, unfortunately, there's no way to split them within a BATS run context (including run_podman). It's a BATS design tradeoff that works really well for the 95% case. The way I rewrote the test in this PR, I check for both the warning and some stdout; but do not (and cannot) check that the warning truly goes to stderr.

@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 24, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 24, 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.

6 participants