Fix insidious quoting bug in build test script (#772)

Summary:
This patch fixes a particularly sneaky bug. Our test script contains a
literal backtick inside single quotes. This is generally not a problem,
because backticks inside single quotes do nothing. But the contents of
the single quotes are interpreted as Bash by our test runner, and at
that time the single quotes are expanded to a command substitution.
Therefore, `grep` is invoked as if writing

    grep -e "warning: running $(yarn backend)"

at the CLI. This will actually invoke `yarn backend`!

The magnificent aspect of this bug is that it both makes the test script
slower by about ten seconds _and_ completely and silently defeats the
assertion in which it’s contained. The output of `yarn backend` contains
several blank lines. Therefore, one of the literal patterns to `grep`
contains a blank line. This causes `grep` to match _every_ line in the
error file, regardless of whether it is one of the intended messages.

This patch is the 666th PR to SourceCred. In my opinion, it deserves
this dubious honor.

Test Plan:
Note that `yarn test --full` works, but fails if one of the expected
error message patterns is deleted or munged.

Confirm the behavior by prepending `echo backend >>/tmp/log &&` to the
`yarn backend` script in `package.json`, noting that the resulting log
file contains four lines before this patch and two lines after it.
(Don’t forget to delete/clear the log file before invocations.)

Confirm the behavior of `grep` by writing:

```shell
$ printf 'things went wrong!\n' >err
$ printf 'wat\n\nwot\n' >patterns
$ grep -vF -e "okay" -e "warn: `cat patterns`" err; echo $?
1
$ printf 'wat\nwot\n' >patterns  # no empty line
$ grep -vF -e "okay" -e "warn: `cat patterns`" err; echo $?
things went wrong!
0
```

wchargin-branch: fix-build-test-quoting
This commit is contained in:
William Chargin 2018-09-05 10:53:05 -07:00 committed by GitHub
parent 1816b13525
commit d2727c01ba
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 1 additions and 1 deletions

View File

@ -152,7 +152,7 @@ run_build() {
run '"${flags}"' 2>err &&
test_must_fail grep -vF \
-e "Removing contents of build directory: " \
-e "warn: running `yarn backend`" \
-e "warn: running \`yarn backend\`" \
-e "warn: if this offends you" \
-e "info: loading repository" \
err &&