~emersion/mrsh-dev

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
1

[PATCH] Overhaul conformance tests

Details
Message ID
<20200205180513.19410-1-sir@cmpwn.com>
DKIM signature
missing
Download raw message
Patch: +141 -80
This introduces comprehensive conformance testing for section 2.2 of the
POSIX shell standard. However, note that mrsh does not currently pass
these new tests. For what it's worth, neither does dash.
---
 meson_options.txt                             |  7 ++
 test/conformance/2.2-quoted-characters.sh     | 36 +++++++++
 test/conformance/2.2-quoted-characters.stdout | 14 ++++
 .../2.2.2-nested-single-quotes.fail.sh        |  3 +
 .../conformance/2.2.3-alias-expansion.fail.sh |  4 +
 ...ackquote-nonterminated-dquote.undefined.sh |  1 +
 ...ackquote-nonterminated-squote.undefined.sh |  1 +
 ...quote-nonterminated-backquote.undefined.sh |  1 +
 test/conformance/harness.sh                   | 21 +++++
 test/conformance/if.sh                        | 78 -------------------
 test/conformance/meson.build                  | 51 ++++++++++++
 test/meson.build                              |  4 +-
 12 files changed, 141 insertions(+), 80 deletions(-)
 create mode 100644 test/conformance/2.2-quoted-characters.sh
 create mode 100644 test/conformance/2.2-quoted-characters.stdout
 create mode 100644 test/conformance/2.2.2-nested-single-quotes.fail.sh
 create mode 100644 test/conformance/2.2.3-alias-expansion.fail.sh
 create mode 100644 test/conformance/2.2.3-backquote-nonterminated-dquote.undefined.sh
 create mode 100644 test/conformance/2.2.3-backquote-nonterminated-squote.undefined.sh
 create mode 100644 test/conformance/2.2.3-dquote-nonterminated-backquote.undefined.sh
 create mode 100644 test/conformance/harness.sh
 delete mode 100644 test/conformance/if.sh
 create mode 100644 test/conformance/meson.build

diff --git a/meson_options.txt b/meson_options.txt
index edfc866..8459089 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -26,3 +26,10 @@ option(
	value: 'sh',
	description: 'Reference shell used in tests',
)

option(
	'test-undefined-behavior',
	type: 'boolean',
	value: true,
	description: 'Run tests that assert undefined behavior fails',
)
diff --git a/test/conformance/2.2-quoted-characters.sh b/test/conformance/2.2-quoted-characters.sh
new file mode 100644
index 0000000..68fe06e
--- /dev/null
+++ b/test/conformance/2.2-quoted-characters.sh
@@ -0,0 +1,36 @@
# 2.2.1 Escape Character (Backslash)
# The following must be quoted:
printf '%s\n' \|\&\;\<\>\(\)\$\`\\\"\'
# The following must be quoted only under certain conditions:
printf '%s\n' *?[#˜=%
# Escaping whitespace:
printf 'arg one: %s; arg two: %s\n' with\ space with\	tab
printf 'arg one: %s; arg two: %s\n' without \
	newline
printf 'arg one: %s; arg two: %s\n' without\
whitespace 'arg two'

# 2.2.2 Single quotes
printf '%s\n' '|&;<>()$`\"'

# 2.2.3 Double quotes
printf '%s\n' "|&;<>()'\""

# Parameter & arithmetic expansion should work:
lval=2
rval=3
printf '%s\n' "$lval + ${rval} = $((lval+rval))"
# Command expansion should work
printf '%s\n' "cmd 1: $(echo "cmd 1")"
printf '%s\n' "cmd 2: `echo "cmd 2"`"
# Command expansion should work, recursively
printf '%s\n' "cmd 3: $(echo $(echo "cmd 3"))"

# Backquotes should work
printf '%s\n' "cmd 4: `echo "cmd 4"`"

# Backslashes should escape the following:
printf '%s\n' "\$\`\"\\\
test"
# But not the following:
printf '%s\n' "\|\&\;\<\>\(\)\'"
diff --git a/test/conformance/2.2-quoted-characters.stdout b/test/conformance/2.2-quoted-characters.stdout
new file mode 100644
index 0000000..5815032
--- /dev/null
+++ b/test/conformance/2.2-quoted-characters.stdout
@@ -0,0 +1,14 @@
|&;<>()$`\"'
*?[#˜=%
arg one: with space; arg two: space	tab
arg one: without; arg two: newline
arg one: withoutwhitespace; arg two: arg two
|&;<>()$`\"
|&;<>()'"
2 + 3 = 5
cmd 1: cmd 1
cmd 2: cmd 2
cmd 3: cmd 3
cmd 4: cmd 4
$`"\test
\|\&\;\<\>\(\)\'
diff --git a/test/conformance/2.2.2-nested-single-quotes.fail.sh b/test/conformance/2.2.2-nested-single-quotes.fail.sh
new file mode 100644
index 0000000..7ea5498
--- /dev/null
+++ b/test/conformance/2.2.2-nested-single-quotes.fail.sh
@@ -0,0 +1,3 @@
# 2.2.2 Single quotes
# Single quotes cannot contain single quotes
printf '%s\n' '''
diff --git a/test/conformance/2.2.3-alias-expansion.fail.sh b/test/conformance/2.2.3-alias-expansion.fail.sh
new file mode 100644
index 0000000..4d87b44
--- /dev/null
+++ b/test/conformance/2.2.3-alias-expansion.fail.sh
@@ -0,0 +1,4 @@
# 2.2.3 Double quotes
# Should NOT apply alias expansion rules when finding the )
alias myalias="echo )"
printf '%s\n' "$(myalias arg-two)"
diff --git a/test/conformance/2.2.3-backquote-nonterminated-dquote.undefined.sh b/test/conformance/2.2.3-backquote-nonterminated-dquote.undefined.sh
new file mode 100644
index 0000000..37d2cab
--- /dev/null
+++ b/test/conformance/2.2.3-backquote-nonterminated-dquote.undefined.sh
@@ -0,0 +1 @@
printf '%s\n' "cmd: `echo "`"
diff --git a/test/conformance/2.2.3-backquote-nonterminated-squote.undefined.sh b/test/conformance/2.2.3-backquote-nonterminated-squote.undefined.sh
new file mode 100644
index 0000000..72b03a8
--- /dev/null
+++ b/test/conformance/2.2.3-backquote-nonterminated-squote.undefined.sh
@@ -0,0 +1 @@
printf '%s\n' "cmd: `echo '`"
diff --git a/test/conformance/2.2.3-dquote-nonterminated-backquote.undefined.sh b/test/conformance/2.2.3-dquote-nonterminated-backquote.undefined.sh
new file mode 100644
index 0000000..e495ca5
--- /dev/null
+++ b/test/conformance/2.2.3-dquote-nonterminated-backquote.undefined.sh
@@ -0,0 +1 @@
printf '%s\n' "`echo"
diff --git a/test/conformance/harness.sh b/test/conformance/harness.sh
new file mode 100644
index 0000000..ebe47b4
--- /dev/null
+++ b/test/conformance/harness.sh
@@ -0,0 +1,21 @@
#!/bin/sh
dir=$(dirname "$0")
testcase="$1"
stdout="${testcase%%.sh}.stdout"

# Set TEST_SHELL to quickly compare the conformance of different shells
mrsh=${TEST_SHELL:-$MRSH}

actual_out=$("$mrsh" "$testcase")
actual_ret=$?

if [ -f "$stdout" ]
then
	stdout="$(cat "$stdout")"
	if [ "$stdout" != "$actual_out" ]
	then
		exit 1
	fi
fi

exit $actual_ret
diff --git a/test/conformance/if.sh b/test/conformance/if.sh
deleted file mode 100644
index 0be4160..0000000
--- a/test/conformance/if.sh
@@ -1,78 +0,0 @@
#!/bin/sh
# Reference stdout:
# pass
# pass
# pass
# pass
# pass
# pass

echo "-> if..fi with true condition"
if true
then
	echo pass
fi

echo "-> if..fi with false condition"
if false
then
	echo >&2 "fail: This branch should not have run" && exit 1
fi
echo pass

echo "-> if..else..fi with true condition"
if true
then
	echo pass
else
	echo >&2 "fail: This branch should not have run" && exit 1
fi

echo "-> if..else..fi with false condition"
if false
then
	echo >&2 "fail: This branch should not have run" && exit 1
else
	echo pass
fi

echo "-> if..else..fi with true condition"
if true
then
	echo pass
else
	echo >&2 "fail: This branch should not have run" && exit 1
fi

echo "-> if..else..elif..fi with false condition"
if false
then
	echo >&2 "fail: This branch should not have run" && exit 1
elif true
then
	echo pass
else
	echo >&2 "fail: This branch should not have run" && exit 1
fi

echo "-> test exit status"
if true
then
	( exit 10 )
else
	( exit 20 )
fi
[ $# -eq 10 ] || { echo >&2 "fail: Expected status code = 10" && exit 1; }
if false
then
	( exit 10 )
else
	( exit 20 )
fi
[ $# -eq 20 ] || { echo >&2 "fail: Expected status code = 20" && exit 1; }

echo "-> test alternate syntax"
# These tests are only expected to parse, they do not make assertions
if true; then true; fi
if true; then true; else true; fi
if true; then true; elif true; then true; else true; fi
diff --git a/test/conformance/meson.build b/test/conformance/meson.build
new file mode 100644
index 0000000..d5bb280
--- /dev/null
+++ b/test/conformance/meson.build
@@ -0,0 +1,51 @@
harness = find_program('./harness.sh')

test_files = [
	'2.2-quoted-characters.sh',
]

failures = [
	'2.2.2-nested-single-quotes.fail.sh',
	'2.2.3-alias-expansion.fail.sh',
]

undefined = [
	'2.2.3-backquote-nonterminated-squote.undefined.sh',
	'2.2.3-backquote-nonterminated-dquote.undefined.sh',
	'2.2.3-dquote-nonterminated-backquote.undefined.sh',
]

testenv = [
	'MRSH=@0@'.format(mrsh_exe.full_path()),
]

foreach test_file : test_files
	test(
		test_file,
		harness,
		env: testenv,
		args: [join_paths(meson.current_source_dir(), test_file)],
	)
endforeach

foreach test_file : failures
	test(
		test_file,
		harness,
		env: testenv,
		args: [join_paths(meson.current_source_dir(), test_file)],
		should_fail: true,
	)
endforeach

if get_option('test-undefined-behavior')
	foreach test_file : undefined
		test(
			test_file,
			harness,
			env: testenv,
			args: [join_paths(meson.current_source_dir(), test_file)],
			should_fail: true,
		)
	endforeach
endif
diff --git a/test/meson.build b/test/meson.build
index fd8aefb..1f05ede 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -2,8 +2,6 @@ harness = find_program('./harness.sh')
ref_sh = find_program(get_option('reference-shell'), required: false)

test_files = [
	'conformance/if.sh',

	'arithm.sh',
	'async.sh',
	'case.sh',
@@ -32,3 +30,5 @@ foreach test_file : test_files
		args: [join_paths(meson.current_source_dir(), test_file)],
	)
endforeach

subdir('conformance')
-- 
2.25.0
Details
Message ID
<enwnDC4AvzoeIcHdCg7cyRo0xOS-U7n05Pxl-lyKzRAyOq6hoTeJeh0FqQj0wwU3I3Bm64VhnyRk6TeG13O6IWpuIvEWNokE4641y2eROCA=@emersion.fr>
In-Reply-To
<20200205180513.19410-1-sir@cmpwn.com> (view parent)
DKIM signature
missing
Download raw message
> This introduces comprehensive conformance testing for section 2.2 of the
> POSIX shell standard. However, note that mrsh does not currently pass
> these new tests. For what it's worth, neither does dash.

Thanks a lot for this new test infrastructure. I really like the idea. Pushed
with some minor edits.

> diff --git a/test/conformance/2.2-quoted-characters.stdout b/test/conformance/2.2-quoted-characters.stdout
> new file mode 100644
> index 0000000..5815032
> --- /dev/null
> +++ b/test/conformance/2.2-quoted-characters.stdout
> @@ -0,0 +1,14 @@
> +|&;<>()$`\"'
> +*?[#˜=%
> +arg one: with space; arg two: space	tab

Typo: "space<tab>tab" should be "with<tab>tab"

> +arg one: without; arg two: newline
> +arg one: withoutwhitespace; arg two: arg two
> +|&;<>()$`\"
> +|&;<>()'"
> +2 + 3 = 5
> +cmd 1: cmd 1
> +cmd 2: cmd 2
> +cmd 3: cmd 3
> +cmd 4: cmd 4
> +$`"\test
> +\|\&\;\<\>\(\)\'

[…]

> diff --git a/test/conformance/2.2.3-alias-expansion.fail.sh b/test/conformance/2.2.3-alias-expansion.fail.sh
> new file mode 100644
> index 0000000..4d87b44
> --- /dev/null
> +++ b/test/conformance/2.2.3-alias-expansion.fail.sh
> @@ -0,0 +1,4 @@
> +# 2.2.3 Double quotes
> +# Should NOT apply alias expansion rules when finding the )
> +alias myalias="echo )"
> +printf '%s\n' "$(myalias arg-two)"

A conforming implementation shouldn't perform the alias substitution when
finding the ), however it's not clear whether it should perform the alias
substitution or not before executing the nested command.

It seems like the idea of the test is that the shell should not perform it.
dash performs the substitution, bash doesn't.

If the substitution is not performed, the command substitution will fail (with
myalias not found). However the printf will overwrite the failed status code of
the command substitution with 0. So the script will always return 0.

Changed it to:

    var="$(myalias arg-two)"

Anyway, I disabled the test because of [1].

How do you interpret the spec here? Is section 2.2.3 only referring to alias
expansion when finding the ), or alias expansion in command substitutions in
general?

[1]: https://github.com/emersion/mrsh/issues/145
Review patch Export thread (mbox)