| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
The test suite has been updated to use the 'test_grep' helper instead
of bare 'grep' for test assertions, allowing file contents to be
printed on failure for easier debugging. A new 'greplint' linter has
been introduced to detect and prevent new bare 'grep' assertions from
being added to the test suite.
* mm/test-grep-lint:
t: add greplint to detect bare grep assertions
t: convert grep assertions to test_grep
t: fix Lexer line count for $() inside double-quoted strings
t: extract chainlint's parser into shared module
t: fix grep assertions missing file arguments
t/README: document test_grep helper
|
|
Replace bare grep with test_grep in test assertions across the
suite, including sourced test helpers (lib-*.sh, *-tests.sh).
test_grep prints the contents of the file being searched on
failure, making debugging easier than a bare grep which fails
silently.
Only assertion-style greps are converted: grep used as a filter
in pipelines, command substitutions, conditionals, or with
redirected I/O is left as-is with a "# lint-ok" annotation.
Existing '! test_grep' calls are rewritten to 'test_grep !' so
that the diagnostic output is preserved on failure.
test_grep requires the file it reads to exist, so '! grep'
assertions that inspect a file whose presence is conditional need
care. In t5537 the '.git/shallow' file is still present after the
repack (the client remains shallow), so the assertion is
converted like any other. In t1400 the '.git/packed-refs' file
exists only with the files backend, so its check is guarded with a
REFFILES prerequisite; the backend-agnostic 'git show-ref' check
that follows still runs under every backend. In t7450 'git~2' is
the NTFS 8.3 short name of a '..git' file and only exists
when 8.3 short-name generation is enabled, so its check is guarded
with a 'test -f' on the path and uses test_grep inside the guard,
the same shape as t1400 (a plain test_grep would BUG when the
short name is absent).
The conversion was generated using a grep-assertion linter
(greplint.pl, added in the following commit) to identify bare
grep calls at command position. To reproduce, from the t/
directory:
# Step 1: annotate the two data-filter greps (grep produces
# data, not a verdict) so the linter skips them.
sed -i '/grep -vf before commits\.raw/s/$/ # lint-ok: data filter/' \
t5326-multi-pack-bitmaps.sh
sed -i '/grep -E "^\[0-9a-f\].*|| :/s/$/ # lint-ok: data filter/' \
t5702-protocol-v2.sh
# Step 1b: two '! grep' assertions need more than a mechanical
# conversion; handle them by hand before the linter-driven steps
# below so it leaves them alone.
#
# t1400: '.git/packed-refs' is absent under reftable, so guard the
# check with REFFILES (a plain test_grep would BUG on the missing
# file):
#
# git update-ref -d HEAD $B &&
# - ! grep "$m" .git/packed-refs &&
# + if test_have_prereq REFFILES
# + then
# + test_grep ! "$m" .git/packed-refs
# + fi &&
# test_must_fail git show-ref --verify -q $m
#
# t7450: git~2 is an NTFS 8.3 short name that exists only when
# short-name generation is enabled, so guard the check on its
# presence with 'test -f' and note in a comment why the path can
# be absent (a plain test_grep would BUG when it is):
#
# - ! grep gitdir squatting-clone/d/a/git~2
# + if test -f squatting-clone/d/a/git~2
# + then
# + test_grep ! gitdir squatting-clone/d/a/git~2
# + fi
# Step 2: reorder pre-existing '! test_grep' to 'test_grep !'
# (must come before steps 3-4 so greplint does not see them)
sed -i 's/! test_grep/test_grep !/' t0031-lockfile-pid.sh
sed -i 's/! test_grep/test_grep !/' t5300-pack-object.sh
sed -i 's/! test_grep/test_grep !/' t5319-multi-pack-index.sh
# Step 3: convert '! grep' -> 'test_grep !'
perl greplint.pl *.sh 2>&1 | cut -d: -f1,2 |
while IFS=: read f l; do
sed -i "${l}s/! *grep/test_grep !/" "$f"
done
# Step 4: convert remaining 'grep' -> 'test_grep'
perl greplint.pl *.sh 2>&1 | cut -d: -f1,2 |
while IFS=: read f l; do
sed -i "${l}s/grep/test_grep/" "$f"
done
To verify, run: make -C t test-greplint
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
There are some typos in the documentation, comments, etc.
Fix them via codespell, and then adjust the "dump" files
used by the subversion tests to match the updated contents.
Signed-off-by: Andrew Kreimer <algonell@gmail.com>
[dscho noticed and fixed the problems in svn test]
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
[jc did final assembling of the three patches]
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
After we have freed the file pair, we should set the queue reference to null.
When computing a diff in a partial clone, there is a chance that we
could trigger a prefetch of missing objects when there are freed entries in
the global diff queue due to break-rewrites detection. The segfault only occurs
if an entry has been freed by break-rewrites and there is an entry
to be prefetched.
There is a new test in t4067 that trigger the segmentation fault that results
in this case. The test explicitly fetch the necessary blobs to trigger the
break rewrites, some blobs are left to be prefetched.
The fix is to set the queue pointer to NULL after it is freed, the prefetch
will skip NULL entries.
Signed-off-by: Han Young <hanyang.tony@bytedance.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
When computing a diff in a partial clone, there is a chance that we
could trigger a prefetch of missing objects at the same time as we are
freeing entries from the global diff queue. This is difficult to
reproduce, as we need to have some objects be freed from the queue
before triggering the prefetch of missing objects. There is a new test
in t4067 that does trigger the segmentation fault that results in this
case.
The fix is to set the queue pointer to NULL after it is freed, and then
to be careful about NULL values in the prefetch.
The more elaborate explanation is that within diffcore_std(), we may
skip the initial prefetch due to the output format (--name-only in the
test) and go straight to diffcore_skip_stat_unmatch(). In that method,
the index entries that have been invalidated by path changes show up as
entries but may be deleted because they are not actually content diffs
and only newer timestamps than expected. As those entries are deleted,
later entries are checked with diff_filespec_check_stat_unmatch(), which
uses diff_queued_diff_prefetch() as the missing_object_cb in its diff
options. That can trigger downloading missing objects if the appropriate
scenario occurs to trigger a call to diff_popoulate_filespec(). It's
finally within that callback to diff_queued_diff_prefetch() that the
segfault occurs.
The test was hard to find because it required some real differences,
some not-different files that had a newer modified time, and the order
of those files alphabetically was important to trigger the deletion
before the prefetch was triggered.
I briefly considered a "lock" member for the diff queue, but it was a
much larger diff and introduced many more possible error scenarios.
Signed-off-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Now that the default value for TEST_PASSES_SANITIZE_LEAK is `true` there
is no longer a need to have that variable declared in all of our tests.
Drop it.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
As a way to assert the path 'foo' is missing, "! test_path_exists
foo" is a poor way to do so, as the helper is designed to complain
when 'foo' is missing, but the intention of the author who used
negated form was to make sure it does not exist. This does not
help debugging the tests.
Use test_path_is_missing instead, which is a more appropriate helper.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The "new_pack" we allocate in check_connected() wasn't being
free'd. Let's do that before we return from the function. This has
leaked ever since "new_pack" was added to this function in
c6807a40dcd (clone: open a shortcut for connectivity check,
2013-05-26).
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
|
|
To prepare for the default value of `protocol.file.allow` to change to
"user", ensure tests that rely on local submodules can initialize them
over the file protocol.
Tests that only need to interact with submodules in a limited capacity
have individual Git commands annotated with the appropriate
configuration via `-c`. Tests that interact with submodules a handful of
times use `test_config_global` instead. Test scripts that rely on
submodules throughout use a `git config --global` during a setup test
towards the beginning of the script.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
|
|
Teach Git to lazy-fetch missing objects in a subprocess instead of doing
it in-process. This allows any fatal errors that occur during the fetch
to be isolated and converted into an error return value, instead of
causing the current command being run to terminate.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
95acf11a3d ("diff: restrict when prefetching occurs", 2020-04-07) taught
diff to prefetch blobs in a more limited set of situations. These
limited situations include when the output format requires blob data,
and when inexact rename detection is needed.
There is an existing test case that tests inexact rename detection, but
it also uses an output format that requires blob data, resulting in the
inexact-rename-detection-only code not being tested. Update this test to
use the raw output format, which does not require blob data.
Thanks to Derrick Stolee for noticing this lapse in code coverage and
for doing the preliminary analysis [1].
[1] https://lore.kernel.org/git/853759d3-97c3-241f-98e1-990883cd204e@gmail.com/
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Commit 7fbbcb21b1 ("diff: batch fetching of missing blobs", 2019-04-08)
optimized "diff" by prefetching blobs in a partial clone, but there are
some cases wherein blobs do not need to be prefetched. In these cases,
any command that uses the diff machinery will unnecessarily fetch blobs.
diffcore_std() may read blobs when it calls the following functions:
(1) diffcore_skip_stat_unmatch() (controlled by the config variable
diff.autorefreshindex)
(2) diffcore_break() and diffcore_merge_broken() (for break-rewrite
detection)
(3) diffcore_rename() (for rename detection)
(4) diffcore_pickaxe() (for detecting addition/deletion of specified
string)
Instead of always prefetching blobs, teach diffcore_skip_stat_unmatch(),
diffcore_break(), and diffcore_rename() to prefetch blobs upon the first
read of a missing object. This covers (1), (2), and (3): to cover the
rest, teach diffcore_std() to prefetch if the output type is one that
includes blob data (and hence blob data will be required later anyway),
or if it knows that (4) will be run.
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
In 7fbbcb21b1 ("diff: batch fetching of missing blobs", 2019-04-08),
diff was taught to batch the fetching of missing objects when operating
on a partial clone, but was not taught to refrain from fetching
GITLINKs. Teach diff to check if an object is a GITLINK before including
it in the set to be fetched.
(As stated in the commit message of that commit, unpack-trees was also
taught a similar thing prior, but unpack-trees correctly checks for
GITLINK before including objects in the set to be fetched.)
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
When running a command like "git show" or "git diff" in a partial clone,
batch all missing blobs to be fetched as one request.
This is similar to c0c578b33c ("unpack-trees: batch fetching of missing
blobs", 2017-12-08), but for another command.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
