-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
tests for exit status on podman run --rm #3804
Conversation
I've confirmed that these tests fail when run against podman < d3a4331 |
Nice clever work on this. |
Should get matching tests into integration testing
…On Tue, Aug 13, 2019, 13:23 Daniel J Walsh ***@***.***> wrote:
Nice clever work on this.
LGTM
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#3804>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AB3AOCHATJA7UEFIGLDT3XLQELUX5ANCNFSM4ILMVNJA>
.
|
New tests here LGTM though
…On Tue, Aug 13, 2019, 14:08 Matthew Heon ***@***.***> wrote:
Should get matching tests into integration testing
On Tue, Aug 13, 2019, 13:23 Daniel J Walsh ***@***.***>
wrote:
> Nice clever work on this.
> LGTM
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> <#3804>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AB3AOCHATJA7UEFIGLDT3XLQELUX5ANCNFSM4ILMVNJA>
> .
>
|
Just need to get them to pass... :^) |
OOF! I thought I had tested podman-remote; turns out I fat-fingered my test. Looks like we have the same broken-exit-status issue with podman-remote. Should I reopen #3795 or open a new one? |
Prooobably a separate issue... I'd open a new issue for it |
Actually - did I sort of suspect the answer is "no" |
@mheon I know that I had to wire a bunch of stuff together to get exec exit codes. I'd believe --rm exit codes were never implemented. |
|
@edsantiago I meant specifically exit codes with |
(Because I'm fairly certain our mitigation for missing exit codes only ever worked locally - the files never existed on the remote system) |
Since we've never had tests, and I've never had a working podman-remote on RHEL8 to run docker-autotest on, I don't know. I still kind of think it might be a good idea to have them, but I can |
Ummm. |
/retest |
...and on a container killed by 'podman rm -f'. See containers#3795 Disable when testing podman-remote; see containers#3808 Signed-off-by: Ed Santiago <[email protected]>
Rebased, and disabled the new tests in podman-remote mode. Will enable if/when #3808 gets fixed. Tests finally green after hours of clicking Re-run on flakes. |
We need to talk about flakes sometime... If I remember, I'll bring it up tomorrow at scrum. |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: edsantiago, mheon 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 |
/lgtm |
...and on a container killed by 'podman rm -f'. See #3795
Signed-off-by: Ed Santiago [email protected]