Improve escaping in unittest failure message (#320)

This commit is contained in:
Samuel Freilich 2021-10-04 12:03:48 -04:00 committed by GitHub
parent b2ed61686e
commit 506c17293e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 162 additions and 18 deletions

View File

@ -36,3 +36,4 @@ Typical usage:
| targets | A list of targets to ensure build. | none |
| kwargs | The <a href="https://docs.bazel.build/versions/main/be/common-definitions.html#common-attributes-tests">common attributes for tests</a>. | none |

View File

@ -23,3 +23,4 @@ The test succeeds if the files' contents match.
| file2 | Label of the file to compare to <code>file1</code>. | none |
| kwargs | The <a href="https://docs.bazel.build/versions/main/be/common-definitions.html#common-attributes-tests">common attributes for tests</a>. | none |

View File

@ -50,3 +50,4 @@ runfiles.
| data | list of labels; data dependencies | <code>None</code> |
| kwargs | The &lt;a href="https://docs.bazel.build/versions/main/be/common-definitions.html#common-attributes-tests"&gt;common attributes for tests&lt;/a&gt;. | none |

View File

@ -22,3 +22,4 @@ Runs a binary as a build action.<br/><br/>This rule does not require Bash (unlik
| srcs | Additional inputs of the action.&lt;br/&gt;&lt;br/&gt;These labels are available for &lt;code&gt;$(location)&lt;/code&gt; expansion in &lt;code&gt;args&lt;/code&gt; and &lt;code&gt;env&lt;/code&gt;. | <a href="https://bazel.build/docs/build-ref.html#labels">List of labels</a> | optional | [] |
| tool | The tool to run in the action.&lt;br/&gt;&lt;br/&gt;Must be the label of a *_binary rule, of a rule that generates an executable file, or of a file that can be executed as a subprocess (e.g. an .exe or .bat file on Windows or a binary with executable permission on Linux). This label is available for &lt;code&gt;$(location)&lt;/code&gt; expansion in &lt;code&gt;args&lt;/code&gt; and &lt;code&gt;env&lt;/code&gt;. | <a href="https://bazel.build/docs/build-ref.html#labels">Label</a> | required | |

View File

@ -5,7 +5,8 @@
## unittest_toolchain
<pre>
unittest_toolchain(<a href="#unittest_toolchain-name">name</a>, <a href="#unittest_toolchain-failure_templ">failure_templ</a>, <a href="#unittest_toolchain-file_ext">file_ext</a>, <a href="#unittest_toolchain-join_on">join_on</a>, <a href="#unittest_toolchain-success_templ">success_templ</a>)
unittest_toolchain(<a href="#unittest_toolchain-name">name</a>, <a href="#unittest_toolchain-escape_chars_with">escape_chars_with</a>, <a href="#unittest_toolchain-escape_other_chars_with">escape_other_chars_with</a>, <a href="#unittest_toolchain-failure_templ">failure_templ</a>, <a href="#unittest_toolchain-file_ext">file_ext</a>,
<a href="#unittest_toolchain-join_on">join_on</a>, <a href="#unittest_toolchain-success_templ">success_templ</a>)
</pre>
@ -16,10 +17,12 @@ unittest_toolchain(<a href="#unittest_toolchain-name">name</a>, <a href="#unitte
| Name | Description | Type | Mandatory | Default |
| :-------------: | :-------------: | :-------------: | :-------------: | :-------------: |
| name | A unique name for this target. | <a href="https://bazel.build/docs/build-ref.html#name">Name</a> | required | |
| failure_templ | - | String | required | |
| file_ext | - | String | required | |
| join_on | - | String | required | |
| success_templ | - | String | required | |
| escape_chars_with | Dictionary of characters that need escaping in test failure message to prefix appended to escape those characters. For example, <code>{"%": "%", "&gt;": "^"}</code> would replace <code>%</code> with <code>%%</code> and <code>&gt;</code> with <code>^&gt;</code> in the failure message before that is included in <code>success_templ</code>. | <a href="https://bazel.build/docs/skylark/lib/dict.html">Dictionary: String -> String</a> | optional | {} |
| escape_other_chars_with | String to prefix every character in test failure message which is not a key in <code>escape_chars_with</code> before including that in <code>success_templ</code>. For example, <code>""</code> would prefix every character in the failure message (except those in the keys of <code>escape_chars_with</code>) with <code>\</code>. | String | optional | "" |
| failure_templ | Test script template with a single <code>%s</code>. That placeholder is replaced with the lines in the failure message joined with the string specified in <code>join_with</code>. The resulting script should print the failure message and exit with non-zero status. | String | required | |
| file_ext | File extension for test script, including leading dot. | String | required | |
| join_on | String used to join the lines in the failure message before including the resulting string in the script specified in <code>failure_templ</code>. | String | required | |
| success_templ | Test script generated when the test passes. Should exit with status 0. | String | required | |
<a name="#analysistest.make"></a>
@ -27,7 +30,8 @@ unittest_toolchain(<a href="#unittest_toolchain-name">name</a>, <a href="#unitte
## analysistest.make
<pre>
analysistest.make(<a href="#analysistest.make-impl">impl</a>, <a href="#analysistest.make-expect_failure">expect_failure</a>, <a href="#analysistest.make-attrs">attrs</a>, <a href="#analysistest.make-fragments">fragments</a>, <a href="#analysistest.make-config_settings">config_settings</a>)
analysistest.make(<a href="#analysistest.make-impl">impl</a>, <a href="#analysistest.make-expect_failure">expect_failure</a>, <a href="#analysistest.make-attrs">attrs</a>, <a href="#analysistest.make-fragments">fragments</a>, <a href="#analysistest.make-config_settings">config_settings</a>,
<a href="#analysistest.make-extra_target_under_test_aspects">extra_target_under_test_aspects</a>)
</pre>
Creates an analysis test rule from its implementation function.
@ -66,6 +70,7 @@ Recall that names of test rules must end in `_test`.
| attrs | An optional dictionary to supplement the attrs passed to the unit test's <code>rule()</code> constructor. | <code>{}</code> |
| fragments | An optional list of fragment names that can be used to give rules access to language-specific parts of configuration. | <code>[]</code> |
| config_settings | A dictionary of configuration settings to change for the target under test and its dependencies. This may be used to essentially change 'build flags' for the target under test, and may thus be utilized to test multiple targets with different flags in a single build | <code>{}</code> |
| extra_target_under_test_aspects | An optional list of aspects to apply to the target_under_test in addition to those set up by default for the test harness itself. | <code>[]</code> |
<a name="#analysistest.begin"></a>

View File

@ -36,7 +36,14 @@ TOOLCHAIN_TYPE = "@bazel_skylib//toolchains/unittest:toolchain_type"
_UnittestToolchainInfo = provider(
doc = "Execution platform information for rules in the bazel_skylib repository.",
fields = ["file_ext", "success_templ", "failure_templ", "join_on"],
fields = [
"file_ext",
"success_templ",
"failure_templ",
"join_on",
"escape_chars_with",
"escape_other_chars_with",
],
)
def _unittest_toolchain_impl(ctx):
@ -47,6 +54,8 @@ def _unittest_toolchain_impl(ctx):
success_templ = ctx.attr.success_templ,
failure_templ = ctx.attr.failure_templ,
join_on = ctx.attr.join_on,
escape_chars_with = ctx.attr.escape_chars_with,
escape_other_chars_with = ctx.attr.escape_other_chars_with,
),
),
]
@ -54,10 +63,59 @@ def _unittest_toolchain_impl(ctx):
unittest_toolchain = rule(
implementation = _unittest_toolchain_impl,
attrs = {
"failure_templ": attr.string(mandatory = True),
"file_ext": attr.string(mandatory = True),
"join_on": attr.string(mandatory = True),
"success_templ": attr.string(mandatory = True),
"failure_templ": attr.string(
mandatory = True,
doc = (
"Test script template with a single `%s`. That " +
"placeholder is replaced with the lines in the " +
"failure message joined with the string " +
"specified in `join_with`. The resulting script " +
"should print the failure message and exit with " +
"non-zero status."
),
),
"file_ext": attr.string(
mandatory = True,
doc = (
"File extension for test script, including leading dot."
),
),
"join_on": attr.string(
mandatory = True,
doc = (
"String used to join the lines in the failure " +
"message before including the resulting string " +
"in the script specified in `failure_templ`."
),
),
"success_templ": attr.string(
mandatory = True,
doc = (
"Test script generated when the test passes. " +
"Should exit with status 0."
),
),
"escape_chars_with": attr.string_dict(
doc = (
"Dictionary of characters that need escaping in " +
"test failure message to prefix appended to escape " +
"those characters. For example, " +
'`{"%": "%", ">": "^"}` would replace `%` with ' +
"`%%` and `>` with `^>` in the failure message " +
"before that is included in `success_templ`."
),
),
"escape_other_chars_with": attr.string(
default = "",
doc = (
"String to prefix every character in test failure " +
"message which is not a key in `escape_chars_with` " +
"before including that in `success_templ`. For " +
'example, `"\"` would prefix every character in ' +
"the failure message (except those in the keys of " +
"`escape_chars_with`) with `\\`."
),
),
},
)
@ -336,7 +394,15 @@ def _end(env):
tc = env.ctx.toolchains[TOOLCHAIN_TYPE].unittest_toolchain_info
testbin = env.ctx.actions.declare_file(env.ctx.label.name + tc.file_ext)
if env.failures:
cmd = tc.failure_templ % tc.join_on.join(env.failures)
failure_message_lines = "\n".join(env.failures).split("\n")
escaped_failure_message_lines = [
"".join([
tc.escape_chars_with.get(c, tc.escape_other_chars_with) + c
for c in line.elems()
])
for line in failure_message_lines
]
cmd = tc.failure_templ % tc.join_on.join(escaped_failure_message_lines)
else:
cmd = tc.success_templ

View File

@ -83,10 +83,11 @@ EOF
# Create test files.
mkdir -p testdir
cat > testdir/BUILD <<EOF
cat > testdir/BUILD <<'EOF'
load("//tests:unittest_tests.bzl",
"basic_passing_test",
"basic_failing_test",
"failure_message_test",
"fail_unexpected_passing_test",
"fail_unexpected_passing_fake_rule")
@ -94,10 +95,26 @@ basic_passing_test(name = "basic_passing_test")
basic_failing_test(name = "basic_failing_test")
failure_message_test(
name = "shell_escape_failure_message_test",
message = "Contains $FOO",
)
failure_message_test(
name = "cmd_escape_failure_message_test",
message = "Contains %FOO%",
)
failure_message_test(
name = "eof_failure_message_test",
message = "\nEOF\n more after EOF",
)
fail_unexpected_passing_test(
name = "fail_unexpected_passing_test",
target_under_test = ":fail_unexpected_passing_fake_target",
)
fail_unexpected_passing_fake_rule(
name = "fail_unexpected_passing_fake_target",
tags = ["manual"])
@ -123,6 +140,36 @@ function test_basic_failing_test() {
expect_log "In test _basic_failing_test from //tests:unittest_tests.bzl: Expected \"1\", but got \"2\""
}
function test_shell_escape_failure_message_test() {
local -r pkg="${FUNCNAME[0]}"
create_pkg "$pkg"
bazel test testdir:shell_escape_failure_message_test --test_output=all --verbose_failures \
>"$TEST_log" 2>&1 && fail "Expected test to fail" || true
expect_log 'In test _failure_message_test from //tests:unittest_tests.bzl: Expected "", but got "Contains $FOO"'
}
function test_cmd_escape_failure_message_test() {
local -r pkg="${FUNCNAME[0]}"
create_pkg "$pkg"
bazel test testdir:cmd_escape_failure_message_test --test_output=all --verbose_failures \
>"$TEST_log" 2>&1 && fail "Expected test to fail" || true
expect_log 'In test _failure_message_test from //tests:unittest_tests.bzl: Expected "", but got "Contains %FOO%"'
}
function test_eof_failure_message_test() {
local -r pkg="${FUNCNAME[0]}"
create_pkg "$pkg"
bazel test testdir:eof_failure_message_test --test_output=all --verbose_failures \
>"$TEST_log" 2>&1 && fail "Expected test to fail" || true
expect_log '^ more after EOF'
}
function test_fail_unexpected_passing_test() {
local -r pkg="${FUNCNAME[0]}"
create_pkg "$pkg"

View File

@ -18,8 +18,9 @@ load("//lib:partial.bzl", "partial")
load("//lib:unittest.bzl", "analysistest", "asserts", "unittest")
###################################
####### fail_basic_test ###########
####### basic_failing_test ########
###################################
def _basic_failing_test(ctx):
"""Unit tests for a basic library verification test that fails."""
env = unittest.begin(ctx)
@ -30,6 +31,27 @@ def _basic_failing_test(ctx):
basic_failing_test = unittest.make(_basic_failing_test)
###################################
####### failure_message_test ######
###################################
def _failure_message_test(ctx):
"""Failing unit test with arbitrary content in the message."""
env = unittest.begin(ctx)
if not ctx.attr.message:
unittest.fail(env, "Message must be non-empty.")
asserts.equals(env, "", ctx.attr.message)
return unittest.end(env)
failure_message_test = unittest.make(
_failure_message_test,
attrs = {
"message": attr.string(),
},
)
###################################
####### basic_passing_test ########
###################################

View File

@ -9,6 +9,7 @@ toolchain_type(
unittest_toolchain(
name = "cmd",
escape_chars_with = {"%": "%"},
failure_templ = """@echo off
echo %s
exit /b 1
@ -30,14 +31,13 @@ toolchain(
unittest_toolchain(
name = "bash",
escape_other_chars_with = "\\",
failure_templ = """#!/bin/sh
cat <<'EOF'
%s
EOF
echo %s
exit 1
""",
file_ext = ".sh",
join_on = "\n",
join_on = "\necho ",
success_templ = "#!/bin/sh\nexit 0",
visibility = ["//visibility:public"],
)