On Fri, Nov 17, 2023 at 06:18:45PM +0100, Thomas Haller wrote:
> The "handle" in JSON output is not stable. Sanitize/normalizeit to 1216.
>
> The number is chosen arbitrarily, but it's somewhat unique in the code
> base. So when you see it, you may guess it originates from sanitization.
Valid handles are monotonic starting at 1. Using 0 as a replacement is
too simple?
> Signed-off-by: Thomas Haller <thaller@xxxxxxxxxx>
> ---
> Note that only a few .json-nft files are adjusted, because otherwise the
> patch is too large. Before applying, you need to adjust them all, by
> running `./tests/shell/run-tests.sh -g`.
Just put the bulk change into a second patch?
[...]
> diff --git a/tests/shell/helpers/json-sanitize-ruleset.sh b/tests/shell/helpers/json-sanitize-ruleset.sh
> index 270a6107e0aa..3b66adabf055 100755
> --- a/tests/shell/helpers/json-sanitize-ruleset.sh
> +++ b/tests/shell/helpers/json-sanitize-ruleset.sh
> @@ -6,7 +6,14 @@ die() {
> }
>
> do_sed() {
> - sed '1s/\({"nftables": \[{"metainfo": {"version": "\)[0-9.]\+\(", "release_name": "\)[^"]\+\(", "\)/\1VERSION\2RELEASE_NAME\3/' "$@"
> + # Normalize the "version"/"release_name", otherwise we have to regenerate the
> + # JSON output upon new release.
> + #
> + # Also, "handle" are not stable. Normalize them to 1216 (arbitrarily chosen).
> + sed \
> + -e '1s/\({"nftables": \[{"metainfo": {"version": "\)[0-9.]\+\(", "release_name": "\)[^"]\+\(", "\)/\1VERSION\2RELEASE_NAME\3/' \
> + -e '1s/"handle": [0-9]\+\>/"handle": 1216/g' \
> + "$@"
> }
Why not just drop the whole metainfo object? A dedicated test could
still ensure its existence.
Also, scoping these replacements to line 1 is funny with single line
input. Worse is identifying the change in the resulting diff. Maybe
write a helper in python which lets you more comfortably sanitize input,
sort attributes by key and output pretty-printed?
In general, the long lines in your scripts make them quite hard to read.
Any particular reason why you don't stick to the 80 columns maxim?
Cheers, Phil
