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.

Sign up for GitHub

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

Jump to bottom

test: replace many grep -Fx by shell comparison #13744

Merged
merged 14 commits into from
Jul 11, 2024
Merged

test: replace many grep -Fx by shell comparison #13744

merged 14 commits into from
Jul 11, 2024

Conversation

simondeziel
Copy link
Member

@simondeziel simondeziel commented Jul 11, 2024 •
edited
Loading

This PR is mainly to improve debuggability. Here's why:

# OK, no problem if we get what we expect
echo foo > test-file
cat test-file | grep -Fx "foo"

# Impossible to figure what we got if it's not what we expected
echo bar > test-file
cat test-file | grep -Fx "foo"

Versus:

# OK, no problem if we get what we expect
echo foo > test-file
[ "$(cat test-file)" = "foo" ]

# Easy to figure what we got if it's not what we expected
echo bar > test-file
[ "$(cat test-file)" = "foo" ]

In the last case, it will be easy to figure what was in the test file as we'll see [ "bar" = "foo" ] thanks to the test being run with set -x.

Additional bonus: this caught a bogus invocation of curl where the output was not properly redirected.

simondeziel added 12 commits July 11, 2024 08:08
@simondeziel
test/suites/backup: replace some grep with shell comparisons
ca24ceb 
Signed-off-by: Simon Deziel <[email protected]>
@simondeziel
test/suites/basic: replace some grep with shell comparisons
475130c 
Signed-off-by: Simon Deziel <[email protected]>
@simondeziel
test/suites/config: replace some grep with shell comparisons
26fc285 
Signed-off-by: Simon Deziel <[email protected]>
@simondeziel
test/suites/clustering: replace some grep with shell comparisons
f0b10c1 
Signed-off-by: Simon Deziel <[email protected]>
@simondeziel
test/suites/clustering: remove bogus || false comparisons supposed …
5e2ef1c 
…to succeed

Also use `grep -F` as fingerprints are fixed strings.

Signed-off-by: Simon Deziel <[email protected]>
@simondeziel
test/suites/container_devices_nic_bridged: replace some grep with she…
dd62568 
…ll comparisons

Signed-off-by: Simon Deziel <[email protected]>
@simondeziel
test/suites/devlxd: replace some grep with shell comparisons
b414fbd 
Signed-off-by: Simon Deziel <[email protected]>
@simondeziel
test/suites/devlxd: improve not authorized check for config key
d908ed0 
Signed-off-by: Simon Deziel <[email protected]>
@simondeziel
test/suites/incremental_copy: replace some grep with shell comparisons
de3542a 
Signed-off-by: Simon Deziel <[email protected]>
@simondeziel
test/suites/migration: replace some grep with shell comparisons
7d4ae57 
Signed-off-by: Simon Deziel <[email protected]>
@simondeziel
test/suites/network_zone: replace some grep with shell comparisons
a2a7490 
Signed-off-by: Simon Deziel <[email protected]>
@simondeziel
test/suites/snapshots: replace some grep with shell comparisons
b11091b 
Signed-off-by: Simon Deziel <[email protected]>
github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 11, 2024
View reviewed changes
test/suites/storage_buckets.sh Outdated
! s3cmdrun "${lxd_backend}" "${roAccessKey}" "${roSecretKey}" delpolicy "s3://${bucketPrefix}.foo" || false
s3cmdrun "${lxd_backend}" "${adAccessKey}" "${adSecretKey}" delpolicy "s3://${bucketPrefix}.foo"
curl -sI --insecure o /dev/null -w "%{http_code}" "${bucketURL}/${lxdTestFile}" | grep -Fx "403"
[ "$(curl -sI --insecure o /dev/null -w "%{http_code}" "${bucketURL}/${lxdTestFile}" = "403" ]

Check warning

Code scanning / shellcheck

SC1009 Warning test

The mentioned syntax error was in this double quoted string.
test/suites/storage_buckets.sh Outdated
! s3cmdrun "${lxd_backend}" "${roAccessKey}" "${roSecretKey}" delpolicy "s3://${bucketPrefix}.foo" || false
s3cmdrun "${lxd_backend}" "${adAccessKey}" "${adSecretKey}" delpolicy "s3://${bucketPrefix}.foo"
curl -sI --insecure o /dev/null -w "%{http_code}" "${bucketURL}/${lxdTestFile}" | grep -Fx "403"
[ "$(curl -sI --insecure o /dev/null -w "%{http_code}" "${bucketURL}/${lxdTestFile}" = "403" ]

Check failure

Code scanning / shellcheck

SC1073 Error test

Couldn't parse this command expansion. Fix to allow more checks.
@simondeziel
test/suites/storage_buckets: replace some grep with shell comparisons
cf8975f 
Also fix one of the `curl` call missing `-` when trying to send the output to
`/dev/null`.

Signed-off-by: Simon Deziel <[email protected]>
@simondeziel
test/suites/storage_local_volume_handling: replace some grep with she…
ab7fbb9 
…ll comparisons

Signed-off-by: Simon Deziel <[email protected]>
@simondeziel simondeziel marked this pull request as ready for review July 11, 2024 14:53
@simondeziel simondeziel requested a review from tomponline as a code owner July 11, 2024 14:53
@simondeziel simondeziel requested a review from hamistao July 11, 2024 14:53
@tomponline tomponline merged commit 65869a4 into canonical:main Jul 11, 2024
29 checks passed
@simondeziel simondeziel deleted the less-grep-Fx branch July 11, 2024 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomponline tomponline tomponline approved these changes

@hamistao hamistao Awaiting requested review from hamistao

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@simondeziel @tomponline