tests/int: nits and cleanups #3369
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kolyshkin
force-pushed
the
more-tests-nits
branch
from
5c6b57e to
9aec7f0
Compare
Merged
Contributor
Author
|
As per comments, #3367 (comment), it makes sense to change CGROUP_UNIFIED to CGROUP_V1 and CGROUP_V2. |
kolyshkin
force-pushed
the
more-tests-nits
branch
4 times, most recently
from
92d25b6 to
bd0c4c0
Compare
kolyshkin
force-pushed
the
more-tests-nits
branch
from
bd0c4c0 to
b822bc4
Compare
Contributor
Author
|
Waiting for #3367 to be merged. |
kolyshkin
force-pushed
the
more-tests-nits
branch
from
b822bc4 to
1f72a84
Compare
kolyshkin
force-pushed
the
more-tests-nits
branch
3 times, most recently
from
fbbd8be to
0996d10
Compare
Contributor
Author
|
@tianon PTAL 🙏🏻 |
kolyshkin
force-pushed
the
more-tests-nits
branch
from
0996d10 to
5de7cba
Compare
Strictly speaking, == is for [[ only, not for [ / test, and, unlike =, the right side is a pattern. To avoid confusion, use =. In cases where we compare with empty string, use -z instead. Keep using [[ in some cases since it does not require quoting the left and right side of comparison (I trust shellcheck on that one). This should have no effect (other than the code being a tad more strict). Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This makes it work similar to all the other variables we use as binary flags. The new 'shellcheck disable' is due to a bug in shellcheck (basically, it does not track the scope of variables or execution order, assuming everything is executed as soon as it is seen). Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This test requires both rootless and root, which does not make sense. Remove the rootless part. Fixes: d41a273 Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The variable $ROOTLESS, as set by helpers.bash and used in many places, provides the same value as $EUID which is always set by bash. Since we are using bash, we can rely on $EUID being omnipresent. Modify all uses accordingly, and since the value is known to be a number, omit the quoting. Similarly, replace all uses of $(id -u) to $EUID. Do some trivial cleanups along the way, such as - simplify some if A; then B; to A && B; - do not use [[ instead of [ where not necessary. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin
force-pushed
the
more-tests-nits
branch
from
5de7cba to
d620a40
Compare
tianon
approved these changes
Mar 23, 2022
Contributor
Author
|
@thaJeztah @mrunalp PTAL (this is already blessed by @tianon). |
Contributor
Author
|
@thaJeztah @mrunalp @AkihiroSuda @cyphar PTAL (this is already blessed by @tianon). |
Contributor
Author
|
@thaJeztah @mrunalp @AkihiroSuda @cyphar Very PTAL. Background story: I am working to add some tests and just caught myself redoing these cleanups from scratch. Believe me, it does not feel good at all. |
mrunalp
approved these changes
May 6, 2022
thaJeztah
approved these changes
May 6, 2022
Member
thaJeztah
left a comment
thaJeztah
left a comment
There was a problem hiding this comment.
LGTM
sorry, for some reason missed the pings 🙀
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Based on and currently includes #3367and #3371. Draft until those are merged.This is a set of bash code cleanups for integration tests, mostly done for correctness, uniformity, and readability.
Please review commit by commit, and see individual commit messages for details.