Skip to content

Commit

Permalink
test: Work around ESLint ignored-file error wart
Browse files Browse the repository at this point in the history
This resolves an issue where `tools/test` would fail with an ESLint
error because you'd touched a file that we'd told ESLint to ignore.

Fixes: #5081
  • Loading branch information
gnprice authored and Somena1 committed Nov 4, 2021
1 parent 477020f commit 03cd864
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 10 deletions.
10 changes: 2 additions & 8 deletions .eslintignore
Original file line number Diff line number Diff line change
@@ -1,11 +1,5 @@
# If you run `tools/test` when you've modified a file that's ignored
# here, you might get a warning like this:
# 0:0 warning File ignored because of a matching ignore pattern. […]
#
# Don't worry about that warning; it won't appear in CI, and it won't
# appear on future `tools/test` runs when not editing these files.
# For more discussion, see:
# https://github.com/zulip/zulip-mobile/pull/4430#issuecomment-767297445
# This should be the only .eslintignore file in our tree.
# See apply_eslintignore in tools/test.

# These are purely type definitions, no runtime code. Most of them
# are third-party code, too, so naturally don't match our style.
Expand Down
36 changes: 34 additions & 2 deletions tools/test
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,41 @@ run_native() {
fi
}

# Filter out filenames excluded by our .eslintignore.
#
# Filenames are expected as arguments, and printed, leaving out those that
# should be ignored.
#
# (This is a workaround: the ideal solution would be for ESLint to accept an
# option saying to apply its ignore rules and to not worry if that filters
# out some or all of the files we named, and then we wouldn't need this.)
apply_eslintignore() {
# This uses only the .eslintignore file at the root. It relies on us
# having no other sources of ESLint file ignores: other .eslintignore
# files, `ignorePatterns` in the eslintrc, etc.
#
# But if we do, the worst case is that we'll reintroduce #5081: the
# interactive `tools/test` (never CI) will fail when one of those
# ignored files is touched, because of an awkward CLI choice in ESLint.

# ESLint ignore files are documented as having the gitignore syntax:
# https://eslint.org/docs/user-guide/configuring/ignoring-code
# So we ask Git to interpret the file for us. The main limitation is
# that while we can tell it one file to apply, we can't give it several,
# or tell it to look for .eslintignore files throughout the tree.
#
# The `git check-ignore` command wants to tell us what files have been
# excluded, not included. With `-nv` it gives us both and we can
# extract which is which.
(( $# )) || return 0 # if no arguments, no output
git -c core.excludesFile=.eslintignore check-ignore --no-index -nv "$@" \
| perl -lne 'print if (s/^::\t//)'
}

run_lint() {
(( $# )) || return 0
eslint ${fix:+--fix} --max-warnings=0 "$@"
files=( $(apply_eslintignore "$@") )
(( ${#files[@]} )) || return 0
eslint ${fix:+--fix} --max-warnings=0 "${files[@]}"
}

check_node() {
Expand Down

0 comments on commit 03cd864

Please sign in to comment.