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 tests: new too-many-arguments test #6733

Merged

Conversation

edsantiago
Copy link
Member

...plus a few others. And fixes to actual parsing.

If a command's usage message includes '...' in the
argument list, assume it can take unlimited arguments.
Nothing we can check.

For all others, though, the ALL-CAPS part on the
right-hand side of the usage message will define
an upper bound on the number of arguments accepted
by the command. So in our 'podman --help' test,
generate N+1 args and run that command. We expect
a 125 exit status and a suitably helpful error message.

Not all podman commands or subcommands were checking,
so I fixed that. And, fixed some broken usage messages
(all-caps FLAGS, and '[flags]' at the end of 'ARGS').
Add new checks to the help test to prevent those in
the future.

Plus a little refactoring/cleanup where necessary.

Signed-off-by: Ed Santiago santiago@redhat.com

@edsantiago
Copy link
Member Author

Treating this as a flake:

[+0008s] Errors during downloading metadata for repository 'updates-testing':
[+0008s]   - Status code: 404 for https://mirror.us-midwest-1.nexcess.net/fedora/updates/testing/32/Everything/x86_64/repodata/45c8231e2ef60bf25041b9e8eee7a6fbaaeea7cfe5b6e22a021459a46f0b4590-prestodelta.xml.zck (IP: 208.69.120.125)
[+0008s]   - Status code: 404 for http://mirror.us-midwest-1.nexcess.net/fedora/updates/testing/32/Everything/x86_64/repodata/6c86a85051b1c46bb3ac85df60c133034a94d4c9a0182ba5f813aaf71f77b2bd-primary.xml.zck (IP: 208.69.120.125)
[+0008s]   - Status code: 404 for http://mirror.us-midwest-1.nexcess.net/fedora/updates/testing/32/Everything/x86_64/repodata/45c8231e2ef60bf25041b9e8eee7a6fbaaeea7cfe5b6e22a021459a46f0b4590-prestodelta.xml.zck (IP: 208.69.120.125)
[+0008s]   - Curl error (23): Failed writing received data to disk/application for http://mirror.us-midwest-1.nexcess.net/fedora/updates/testing/32/Everything/x86_64/repodata/8ff7102feb09bc617ed4f0638a4ad0478e4d72d6931c4c52749c9047e0c8b3d6-filelists.xml.zck [Failed writing body (0 != 153)]
[+0008s] Error: Failed to download metadata for repo 'updates-testing': Yum repo downloading error: Downloading error(s): repodata/8ff7102feb09bc617ed4f0638a4ad0478e4d72d6931c4c52749c9047e0c8b3d6-filelists.xml.zck - Download failed: Curl error (23): Failed writing received data to disk/application for http://mirror.us-midwest-1.nexcess.net/fedora/updates/testing/32/Everything/x86_64/repodata/8ff7102feb09bc617ed4f0638a4ad0478e4d72d6931c4c52749c9047e0c8b3d6-filelists.xml.zck [Failed writing body (0 != 153)]

(in special_testing_rootless TEST_REMOTE_CLIENT:false)

@edsantiago
Copy link
Member Author

In in-podman fedora-32, also treating as flake:

Error: unable to pull k8s.gcr.io/pause:3.2: Error reading blob sha256:c74f8866df097496217c9f15efe8f8d3db05d19d678a02d01cc7eaed520bb136: Get "https://storage.googleapis.com/us.artifacts.google-containers.appspot.com/containers/images/sha256:c74f8866df097496217c9f15efe8f8d3db05d19d678a02d01cc7eaed520bb136": dial tcp 172.217.214.128:443: i/o timeout

@edsantiago
Copy link
Member Author

Treating as flake (in build_each_commit):

[+0032s] Errors during downloading metadata for repository 'updates':
[+0032s]   - Status code: 404 for https://mirror.us-midwest-1.nexcess.net/fedora/updates/32/Everything/x86_64/repodata/ae151992d5d1095bfff7a80e0de749172a797374366e840190cf11b253215b05-updateinfo.xml.zck (IP: 208.69.120.125)
[+0032s]   - Status code: 404 for http://mirror.us-midwest-1.nexcess.net/fedora/updates/32/Everything/x86_64/repodata/ae151992d5d1095bfff7a80e0de749172a797374366e840190cf11b253215b05-updateinfo.xml.zck (IP: 208.69.120.125)
[+0032s]   - Status code: 404 for http://mirror.us-midwest-1.nexcess.net/fedora/updates/32/Everything/x86_64/repodata/893c6e023e7d44182837e923a237d8f3451637fa5e64dbbc2a2854267a1812ba-comps-Everything.x86_64.xml.gz (IP: 208.69.120.125)
[+0032s]   - Curl error (23): Failed writing received data to disk/application for https://mirror.us-midwest-1.nexcess.net/fedora/updates/32/Everything/x86_64/repodata/41ce0cfb805c85e6c03be92fa950a53c65b5d2aed232162c226cc21c74515c3d-filelists.xml.zck [Failed writing body (0 != 153)]
[+0032s]   - Status code: 404 for https://mirror.us-midwest-1.nexcess.net/fedora/updates/32/Everything/x86_64/repodata/893c6e023e7d44182837e923a237d8f3451637fa5e64dbbc2a2854267a1812ba-comps-Everything.x86_64.xml.gz (IP: 208.69.120.125)
[+0032s] Error: Failed to download metadata for repo 'updates': Yum repo downloading error: Downloading error(s): repodata/41ce0cfb805c85e6c03be92fa950a53c65b5d2aed232162c226cc21c74515c3d-filelists.xml.zck - Download failed: Curl error (23): Failed writing received data to disk/application for https://mirror.us-midwest-1.nexcess.net/fedora/updates/32/Everything/x86_64/repodata/41ce0cfb805c85e6c03be92fa950a53c65b5d2aed232162c226cc21c74515c3d-filelists.xml.zck [Failed writing body (0 != 153)]

...plus a few others. And fixes to actual parsing.

If a command's usage message includes '...' in the
argument list, assume it can take unlimited arguments.
Nothing we can check.

For all others, though, the ALL-CAPS part on the
right-hand side of the usage message will define
an upper bound on the number of arguments accepted
by the command. So in our 'podman --help' test,
generate N+1 args and run that command. We expect
a 125 exit status and a suitably helpful error message.

Not all podman commands or subcommands were checking,
so I fixed that. And, fixed some broken usage messages
(all-caps FLAGS, and '[flags]' at the end of 'ARGS').
Add new checks to the help test to prevent those in
the future.

Plus a little refactoring/cleanup where necessary.

Signed-off-by: Ed Santiago <santiago@redhat.com>
@rhatdan
Copy link
Member

rhatdan commented Jun 23, 2020

LGTM

@edsantiago
Copy link
Member Author

Thanks. No changes in this last push a few minutes ago, it was just a rebase because CI had been stuck for 3 hours.

@rhatdan
Copy link
Member

rhatdan commented Jun 23, 2020

/approve

@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-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 23, 2020
@edsantiago
Copy link
Member Author

Tests (finally) green. @baude, @jwhonce, @vrothberg, @QiWang19, @mheon, this PR touches code that you've had a hand in. If you have time, I'd appreciate a quick once-over of those parts of the code you recognize (no need to review the BATS stuff, just the changes in argument processing). TIA.

Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

LGTM, amazing bash witchcraft!

I'll let others have a look before merging.

@rhatdan
Copy link
Member

rhatdan commented Jun 24, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 24, 2020
@openshift-merge-robot openshift-merge-robot merged commit c48a542 into containers:master Jun 24, 2020
@edsantiago edsantiago deleted the bats_help_extra_args branch June 24, 2020 13:27
@QiWang19
Copy link
Collaborator

LGTM

@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