When multiple concurrent processes try to update references in a
repository they may try to lock the same lockfiles. This can happen even
when the updates are non-conflicting and can both be applied, so it
doesn't always make sense to abort the transaction immediately. Both the
"loose" and "packed" backends thus have a grace period that they wait
for the lock to be released that can be controlled via the config values
"core.filesRefLockTimeout" and "core.packedRefsTimeout", respectively.
The reftable backend doesn't have such a setting yet and instead fails
immediately when it sees such a lock. But the exact same concepts apply
here as they do apply to the other backends.
Introduce a new "reftable.lockTimeout" config that controls how long we
may wait for a "tables.list" lock to be released. The default value of
this config is 100ms, which is the same default as we have it for the
"loose" backend.
Note that even though we also lock individual tables, this config really
only applies to the "tables.list" file. This is because individual
tables are only ever locked when we already hold the "tables.list" lock
during compaction. When we observe such a lock we in fact do not want to
compact the table at all because it is already in the process of being
compacted by a concurrent process. So applying the same timeout here
would not make any sense and only delay progress.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The error messages from the test script checker have been improved.
* es/chainlint-message-updates:
chainlint: reduce annotation noise-factor
chainlint: make error messages self-explanatory
chainlint: don't be fooled by "?!...?!" in test body
Import clar unit tests framework libgit2 folks invented for our
use.
* ps/clar-unit-test:
Makefile: rename clar-related variables to avoid confusion
clar: add CMake support
t/unit-tests: convert ctype tests to use clar
t/unit-tests: convert strvec tests to use clar
t/unit-tests: implement test driver
Makefile: wire up the clar unit testing framework
Makefile: do not use sparse on third-party sources
Makefile: make hdr-check depend on generated headers
Makefile: fix sparse dependency on GENERATED_H
clar: stop including `shellapi.h` unnecessarily
clar(win32): avoid compile error due to unused `fs_copy()`
clar: avoid compile error with mingw-w64
t/clar: fix compatibility with NonStop
t: import the clar unit testing framework
t: do not pass GIT_TEST_OPTS to unit tests with prove
Bugfixes and leak plugging in "git for-each-ref --format=..." code
paths.
* jk/ref-filter-trailer-fixes:
ref-filter: fix leak with unterminated %(if) atoms
ref-filter: add ref_format_clear() function
ref-filter: fix leak when formatting %(push:remoteref)
ref-filter: fix leak with %(describe) arguments
ref-filter: fix leak of %(trailers) "argbuf"
ref-filter: store ref_trailer_buf data per-atom
ref-filter: drop useless cast in trailers_atom_parser()
ref-filter: strip signature when parsing tag trailers
ref-filter: avoid extra copies of payload/signature
t6300: drop newline from wrapped test title
Another reftable test migrated to the unit-test framework.
* cp/unit-test-reftable-stack:
t-reftable-stack: add test for stack iterators
t-reftable-stack: add test for non-default compaction factor
t-reftable-stack: use reftable_ref_record_equal() to compare ref records
t-reftable-stack: use Git's tempfile API instead of mkstemp()
t: harmonize t-reftable-stack.c with coding guidelines
t: move reftable/stack_test.c to the unit testing framework
The interpret-trailers command failed to recognise the end of the
message when the commit log ends in an incomplete line.
* bl/trailers-and-incomplete-last-line-fix:
interpret-trailers: handle message without trailing newline
In a few corner cases "git diff --exit-code" failed to report
"changes" (e.g., renamed without any content change), which has
been corrected.
* rs/diff-exit-code-fix:
diff: report dirty submodules as changes in builtin_diff()
diff: report copies and renames as changes in run_diff_cmd()
"git verify-pack" and "git index-pack" started dying outside a
repository, which has been corrected.
* ps/index-pack-outside-repo-fix:
builtin/index-pack: fix segfaults when running outside of a repo
"git cat-file" works well with the sparse-index, and gets marked as
such.
* kl/cat-file-on-sparse-index:
builtin/cat-file: mark 'git cat-file' sparse-index compatible
t1092: allow run_on_* functions to use standard input
One-line messages to "die" and other helper functions will get LF
added by these helper functions, but many existing messages had an
unnecessary LF at the end, which have been corrected.
* jk/messages-with-excess-lf-fix:
drop trailing newline from warning/error/die messages
"git pack-refs --auto" for the files backend was too aggressive,
which has been a bit tamed.
* ps/pack-refs-auto-heuristics:
refs/files: use heuristic to decide whether to repack with `--auto`
t0601: merge tests for auto-packing of refs
wrapper: introduce `log2u()`
A data corruption bug when multi-pack-index is used and the same
objects are stored in multiple packfiles has been corrected.
* tb/multi-pack-reuse-fix:
builtin/pack-objects.c: do not open-code `MAX_PACK_OBJECT_HEADER`
pack-bitmap.c: avoid repeated `pack_pos_to_offset()` during reuse
builtin/pack-objects.c: translate bit positions during pack-reuse
pack-bitmap: tag bitmapped packs with their corresponding MIDX
t/t5332-multi-pack-reuse.sh: verify pack generation with --strict
"git verify-pack" and "git index-pack" started dying outside a
repository, which has been corrected.
* ps/index-pack-outside-repo-fix:
builtin/index-pack: fix segfaults when running outside of a repo
"git bundle unbundle" outside a repository triggered a BUG()
unnecessarily, which has been corrected.
* ps/bundle-outside-repo-fix:
bundle: default to SHA1 when reading bundle headers
builtin/bundle: have unbundle check for repo before opening its bundle
The patch parser in "git patch-id" has been tightened to avoid
getting confused by lines that look like a patch header in the log
message.
cf. <Zqh2T_2RLt0SeKF7@tanuki>
* jc/patch-id:
patch-id: tighten code to detect the patch header
patch-id: rewrite code that detects the beginning of a patch
patch-id: make get_one_patchid() more extensible
patch-id: call flush_current_id() only when needed
t4204: patch-id supports various input format
When chainlint detects a problem in a test definition, it highlights the
offending code with a "?!...?!" annotation. The rather curious "?!"
decoration was chosen to draw the reader's attention to the problem area
and to act as a good "needle" when using the terminal's search feature
to "jump" to the next problem.
Later, chainlint learned to color its output when sent to a terminal.
Problem annotations are colored with a red background which stands out
well from surrounding text, thus easily draws the reader's attention.
Together with the preceding change which gave all problem annotations a
uniform "LINT:" prefix, the noisy "?!" decoration has become superfluous
as a search "needle" so omit it when output is colored.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The annotations emitted by chainlint to indicate detected problems are
overly terse, so much so that developers new to the project -- those who
should most benefit from the linting -- may find them baffling. For
instance, although the author of chainlint and seasoned Git developers
may understand that "?!AMP?!" is an abbreviation of "ampersand" and
indicates a break in the &&-chain, this may not be obvious to newcomers.
The "?!LOOP?!" case is particularly serious because that terse single
word does nothing to convey that the loop body should end with
"|| return 1" (or "|| exit 1" in a subshell) to ensure that a failing
command in the body aborts the loop immediately. Moreover, unlike
&&-chaining which is ubiquitous in Git tests, the "|| return 1" idiom is
relatively infrequent, thus may be harder for a newcomer to discover by
consulting nearby code.
Address these shortcomings by emitting human-readable messages which
both explain the problem and give a strong hint about how to correct it.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
As originally implemented, chainlint did not collect structured
information about detected problems. Instead, it merely emitted raw
parse tokens (not the original test text), along with a "?!...?!"
annotation directly into the output stream each time a problem was
discovered. In order to report statistics (in --stats mode) and to
adjust its exit code to indicate success or failure, it merely counts
the number of times "?!...?!" appears in the output stream. An obvious
shortcoming of this approach is that it can be fooled by a legitimate
"?!...?!" sequence in the body of a test (though, only if an actual
problem is detected in the test).
The situation did not improve when 7c04aa7390 (chainlint: colorize
problem annotations and test delimiters, 2022-09-13) colored the
annotations after-the-fact by searching for "?!...?!" in the output
stream and inserting color codes. As above, a shortcoming is that this
approach can incorrectly color a legitimate "?!...?!" sequence in a test
body as if it is an error.
However, when 73c768dae9 (chainlint: annotate original test definition
rather than token stream, 2022-11-08) taught chainlint to output the
original test text verbatim, it started collecting structured
information about detected problems.
Now that it is available, take advantage of the structured problem
information to deterministically count the number of problems detected
and to color the annotations directly, rather than scanning the output
stream for "?!...?!" and performing these operations after-the-fact.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When parsing `%(if)` atoms we expect a few other atoms to exist to
complete it, like `%(then)` and `%(end)`. Whether or not we have seen
these other atoms is tracked in an allocated `if_then_else` structure,
which gets free'd by the `if_then_else_handler()` once we have parsed
the complete conditional expression.
This results in a memory leak when the `%(if)` atom is not terminated
correctly and thus incomplete. We never end up executing its handler and
thus don't end up freeing the structure.
Plug this memory leak by introducing a new `at_end_data_free` callback
function. If set, we'll execute it in `pop_stack_element()` and pass it
the `at_end_data` variable with the intent to free its state. Wire it up
for the `%(if)` atom accordingly.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
After using the ref-filter API, callers should use ref_filter_clear() to
free any used memory. However, there's not a matching function to clear
the ref_format struct.
Traditionally this did not need to be cleaned up, as it was just a way
for the caller to store and pass format options as a single unit. Even
though the parsing step of some placeholders may allocate data, that's
usually inside their "used_atom" structs, which are part of the
ref_filter itself.
But a few placeholders keep data outside of there. The %(ahead-behind)
and %(is-base) parsers both keep a master list of bases, because they
perform a single filtering pass outside of the use of any particular
atom. And since the format parser does not have access to the ref_filter
struct, they store their cross-atom data in the ref_format struct
itself.
And thus when they are finished, the ref_format also needs to be cleaned
up. So let's add a function to do so, and call it from all of the users
of the ref-filter API.
The %(is-base) case is found by running LSan on t6300. After this patch,
the script can now be marked leak-free.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The trailer API takes options via a trailer_opts struct. Some of those
options point to data structures which require extra storage. Those
structures aren't actually embedded in the options struct, but rather we
pass pointers, and the caller is responsible for managing them. This is
a little convoluted, but makes sense since some of them are not even
concrete (e.g., you can pass a filter function and a void data pointer,
but the trailer code doesn't even know what's in the pointer).
When for-each-ref, etc, parse the %(trailers) placeholder, they stuff
the extra data into a ref_trailer_buf struct. But we only hold a single
static global instance of this struct. So if a format string has
multiple %(trailer) placeholders, they'll stomp on each other: the "key"
list will end up with entries for all of them, and the separator buffers
will use the values from whichever was parsed last.
Instead, we should have a ref_trailer_buf for each instance of the
placeholder, and store it alongside the trailer_opts in the used_atom
structure.
And that's what this patch does. Note that we also have to add code to
clean them up in ref_array_clear(). The original code did not bother
cleaning them up, but it wasn't technically a "leak" since they were
still reachable from the static global instance.
Reported-by: Brooke Kuhlmann <brooke@alchemists.io>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
To expand the "%(trailers)" placeholder, we have to feed the commit or
tag body to the trailer API. But that API doesn't know anything about
signatures, and will be confused by a signed tag like this:
the subject
the body
Some-trailer: foo
-----BEGIN PGP SIGNATURE-----
...etc...
because it will start looking for trailers after the signature, and get
stopped walking backwards by the very non-trailer signature lines. So it
thinks there are no trailers.
This problem has existed since %(trailers) was added to the ref-filter
code, but back then trailers on tags weren't something we really
considered (commits don't have the same problem because their signatures
are embedded in the header). But since 066cef7707 (builtin/tag: add
--trailer option, 2024-05-05), we'd generate an object like the above
for "git tag -s --trailer 'Some-trailer: foo' my-tag".
The implementation here is pretty simple: we just make a NUL-terminated
copy of the non-signature part of the tag (which we've already parsed)
and pass it to the trailer API. There are some alternatives I rejected,
at least for now:
- the trailer code already understands skipping past some cruft at the
end of a commit, such as patch dividers. see find_end_of_log_message().
We could teach it to do the same for signatures. But since this is
the only context where we'd want that feature, and since we've already
parsed the object into subject/body/signature here, it seemed easier
to just pass in the truncated message.
- it would be nice if we could just pass in a pointer/len pair to the
trailer API (rather than a NUL-terminated string) to avoid the extra
copy. I think this is possible, since as noted above, the trailer
code already has to deal with ignoring some cruft at the end of the
input. But after an initial attempt at this, it got pretty messy, as
we have to touch a lot of intermediate functions that are also
called in other contexts.
So I went for the simple and stupid thing, at least for now. I don't
think the extra copy overhead will be all that bad. The previous
patch noted that an extra copy seemed to cause about 1-2% slowdown
for something simple like "%(subject)". But here we are only
triggering it for "%(trailers)" (and only when there is a
signature), and the trailer code is a bit allocation-heavy already.
I couldn't measure any difference formatting "%(trailers)" on
linux.git before and after (even though there are not even any
trailers to find).
Reported-by: Brooke Kuhlmann <brooke@alchemists.io>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We don't usually include newlines in test titles, because you get funny
TAP output like:
ok 417 - show good signature with custom format
ok 418 - show good signature with custom format
with ssh
ok 419 - signature atom with grade option and bad signature
where a TAP parser would ignore the extra line anyway, giving the wrong
title. This comes from 26c9c03f0a (ref-filter: add new "signature" atom,
2023-06-04), and I think it was probably just editor line wrapping.
I checked for other cases with:
git grep "test_expect_success [A-Z_,]* '[^']*$"
git grep 'test_expect_success [A-Z_,]* "[^"]*$'
but this was the only hit.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
reftable_stack_init_ref_iterator and reftable_stack_init_log_iterator
as defined by reftable/stack.{c,h} initialize a stack iterator to
iterate over the ref and log records in a reftable stack respectively.
Since these functions are not exercised by any of the existing tests,
add a test for them.
Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In a recent codebase update (commit ae8e378430, merge branch
'ps/reftable-write-options', 2024/05/13) the geometric factor used
in auto-compaction of reftable tables was made configurable. Add
a test to verify the functionality introduced by this update.
Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the current stack tests, ref records are compared for equality
by sometimes using the dedicated function for ref-record comparison,
reftable_ref_record_equal(), and sometimes by explicity comparing
contents of the ref records.
The latter method is undesired because there can exist unequal ref
records with some of the contents being equal. Replace the latter
instances of ref-record comparison with the former. This has the
added benefit of preserving uniformity throughout the test file.
Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
--ours, --theirs, and --union are already supported in `git merge-file`
for automatically resolving conflicts in favor of one version or the
other, instead of leaving conflict markers in the file. Support them in
`git apply -3` as well because the two commands do the same kind of
file-level merges.
In case in the future --ours, --theirs, and --union gain a meaning
outside of three-way-merges, they do not imply --3way but rather must be
specified alongside it.
Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Git's tempfile API defined by $GIT_DIR/tempfile.{c,h} provides
a unified interface for tempfile operations. Since reftable/stack.c
uses this API for all its tempfile needs instead of raw functions
like mkstemp(), make the ported stack test strictly use Git's
tempfile API as well.
A bigger benefit is the fact that we know to clean up the tempfile
in case the test fails because it gets registered and pruned via a
signal handler.
Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Harmonize the newly ported test unit-tests/t-reftable-stack.c
with the following guidelines:
- Single line 'for' statements must omit curly braces.
- Structs must be 0-initialized with '= { 0 }' instead of '= { NULL }'.
- Array sizes and indices should preferably be of type 'size_t' and
not 'int'.
- Function pointers should be passed as 'func' and not '&func'.
While at it, remove initialization for those variables that are
re-used multiple times, like loop variables.
Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
reftable/stack_test.c exercises the functions defined in
reftable/stack.{c, h}. Migrate reftable/stack_test.c to the
unit testing framework. Migration involves refactoring the tests
to use the unit testing framework instead of reftable's test
framework and renaming the tests to be in-line with unit-tests'
standards.
Since some of the tests use set_test_hash() defined by
reftable/test_framework.{c, h} but these files are not
'#included' in the test file, copy this function in the
ported test file.
With the migration of stack test to the unit-tests framework,
"test-tool reftable" becomes a no-op. Hence, get rid of everything
that uses "test-tool reftable" alongside everything that is used
to implement it.
While at it, alphabetically sort the cmds[] list in
helper/test-tool.c by moving the entry for "dump-reftable".
Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The diff machinery has two ways to detect changes to set the exit code:
Just comparing hashes and comparing blob contents. The latter is needed
if certain changes have to be ignored, e.g. with --ignore-space-change
or --ignore-matching-lines. It's enabled by the diff_options flag
diff_from_contents.
The slower mode as never considered submodules (and subrepos) as changes
with --submodule=diff or --submodule=log, which is inconsistent with
--submodule=short (the default). Fix it.
d7b97b7185 (diff: let external diffs report that changes are
uninteresting, 2024-06-09) set diff_from_contents if external diff
programs are allowed. This is the default e.g. for git diff, and so
that change exposed the inconsistency much more widely.
Reported-by: David Hull <david.hull@friendbuy.com>
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The diff machinery has two ways to detect changes to set the exit code:
Just comparing hashes and comparing blob contents. The latter is needed
if certain changes have to be ignored, e.g. with --ignore-space-change
or --ignore-matching-lines. It's enabled by the diff_options flag
diff_from_contents.
The slower mode has never considered copies and renames to be changes,
which is inconsistent with the quicker one. Fix it. Even if we ignore
the file contents (because it's empty or contains only ignored lines),
there's still the meta data change of adding or changing a filename, so
we need to report it in the exit code.
d7b97b7185 (diff: let external diffs report that changes are
uninteresting, 2024-06-09) set diff_from_contents if external diff
programs are allowed. This is the default e.g. for git diff, and so
that change exposed the inconsistency much more widely.
Reported-by: Jorge Luis Martinez Gomez <jol@jol.dev>
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Some large repositories use tags to track a huge list of release
versions. While this choice is costly on the ref advertisement, it is
further wasteful for clients who do not need those tags. Allow clients
to optionally skip the tag advertisement.
This behavior is similar to that of 'git clone --no-tags' implemented in
0dab2468ee (clone: add a --no-tags option to clone without tags,
2017-04-26), including the modification of the remote.origin.tagOpt
config value to include "--no-tags".
One thing that is opposite of the 'git clone' implementation is that
this allows '--tags' as an assumed option, which can be naturally negated
with '--no-tags'. The clone command does not accept '--tags' but allows
"--no-no-tags" as the negation of its '--no-tags' option.
While testing this option, combine the test with the previously untested
'--no-src' option introduced in 4527db8ff8 (scalar: add --[no-]src
option, 2023-08-28).
Signed-off-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Make our codebase compilable with the -Werror=unused-parameter
option.
* jk/unused-parameters:
CodingGuidelines: mention -Wunused-parameter and UNUSED
config.mak.dev: enable -Wunused-parameter by default
compat: mark unused parameters in win32/mingw functions
compat: disable -Wunused-parameter in win32/headless.c
compat: disable -Wunused-parameter in 3rd-party code
t-reftable-readwrite: mark unused parameter in callback function
gc: mark unused config parameter in virtual functions
When git-interpret-trailers is used to add a trailer to a message that
does not end in a trailing newline, the new trailer is added on the line
immediately following the message instead of as a trailer block
separated from the message by a blank line.
For example, if a message's text was exactly "The subject" with no
trailing newline present, `git interpret-trailers --trailer
my-trailer=true` will result in the following malformed commit message:
The subject
my-trailer: true
While it is generally expected that a commit message should end with a
newline character, git-interpret-trailers should not be returning an
invalid message in this case.
Use `strbuf_complete_line` to ensure that the message ends with a
newline character when reading the input.
Signed-off-by: Brian Lyles <brianmlyles@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Our error reporting routines append a trailing newline, and the strings
we pass to them should not include them (otherwise we get an extra blank
line after the message).
These cases were all found by looking at the results of:
git grep -P '[^_](error|error_errno|warning|die|die_errno)\(.*\\n"[,)]' '*.c'
Note that we _do_ sometimes include a newline in the middle of such
messages, to create multiline output (hence our grep matching "," or ")"
after we see the newline, so we know we're at the end of the string).
It's possible that one or more of these cases could intentionally be
including a blank line at the end, but having looked at them all
manually, I think these are all just mistakes.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This change affects how 'git cat-file' works with the index when
specifying an object with the ":<path>" syntax (which will give file
contents from the index).
'git cat-file' expands a sparse index to a full index any time contents
are requested from the index by specifying an object with the ":<path>"
syntax. This is true even when the requested file is part of the sparse
index, and results in much slower 'git cat-file' operations when working
within the sparse index.
Mark 'git cat-file' as not needing a full index, so that you only pay
the cost of expanding the sparse index to a full index when you request
a file outside of the sparse index.
Add tests to ensure both that:
- 'git cat-file' returns the correct file contents whether or not the
file is in the sparse index
- 'git cat-file' expands to the full index any time you request
something outside of the sparse index
Signed-off-by: Kevin Lyles <klyles+github@epic.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The 'run_on_sparse' and 'run_on_all' functions do not work correctly for
commands accepting standard input, because they run the same command
multiple times and the first instance consumes it. This also indirectly
affects 'test_all_match' and 'test_sparse_match'.
To allow these functions to work with commands accepting standard input,
first slurp standard input to a temporary file, and then run the command
with its standard input redirected from the temporary file. This ensures
that each command sees the same contents from its standard input.
Note that this does not impact commands that do not read from standard
input; they continue to ignore it. Additionally, existing uses of the
run_on_* functions do not need to do anything differently, as the
standard input of the test environment is already connected to
/dev/null.
We do not explicitly clean up the input files because they are cleaned
up with the rest of the test repositories and their contents may be
useful for figuring out which command failed when a test case fails.
Signed-off-by: Kevin Lyles <klyles@epic.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Convert the ctype tests to use the new clar unit testing framework.
Introduce a new function `cl_failf()` that allows us to print a
formatted error message, which we can use to point out which of the
characters was classified incorrectly. This results in output like this
on failure:
# start of suite 1: ctype
not ok 1 - ctype::isspace
---
reason: |
Test failed.
0x0d is classified incorrectly: expected 0, got 1
at:
file: 't/unit-tests/ctype.c'
line: 36
function: 'test_ctype__isspace'
---
ok 2 - ctype::isdigit
ok 3 - ctype::isalpha
ok 4 - ctype::isalnum
ok 5 - ctype::is_glob_special
ok 6 - ctype::is_regex_special
ok 7 - ctype::is_pathspec_magic
ok 8 - ctype::isascii
ok 9 - ctype::islower
ok 10 - ctype::isupper
ok 11 - ctype::iscntrl
ok 12 - ctype::ispunct
ok 13 - ctype::isxdigit
ok 14 - ctype::isprint
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Convert the strvec tests to use the new clar unit testing framework.
This is a first test balloon that demonstrates how the testing infra for
clar-based tests looks like.
The tests are part of the "t/unit-tests/bin/unit-tests" binary. When
running that binary with an injected error, it generates TAP output:
# ./t/unit-tests/bin/unit-tests
TAP version 13
# start of suite 1: strvec
ok 1 - strvec::init
ok 2 - strvec::dynamic_init
ok 3 - strvec::clear
not ok 4 - strvec::push
---
reason: |
String mismatch: (&vec)->v[i] != expect[i]
'foo' != 'fo' (at byte 2)
at:
file: 't/unit-tests/strvec.c'
line: 48
function: 'test_strvec__push'
---
ok 5 - strvec::pushf
ok 6 - strvec::pushl
ok 7 - strvec::pushv
ok 8 - strvec::replace_at_head
ok 9 - strvec::replace_at_tail
ok 10 - strvec::replace_in_between
ok 11 - strvec::replace_with_substring
ok 12 - strvec::remove_at_head
ok 13 - strvec::remove_at_tail
ok 14 - strvec::remove_in_between
ok 15 - strvec::pop_empty_array
ok 16 - strvec::pop_non_empty_array
ok 17 - strvec::split_empty_string
ok 18 - strvec::split_single_item
ok 19 - strvec::split_multiple_items
ok 20 - strvec::split_whitespace_only
ok 21 - strvec::split_multiple_consecutive_whitespaces
ok 22 - strvec::detach
1..22
The binary also supports some parameters that allow us to run only a
subset of unit tests or alter the output:
$ ./t/unit-tests/bin/unit-tests -h
Usage: ./t/unit-tests/bin/unit-tests [options]
Options:
-sname Run only the suite with `name` (can go to individual test name)
-iname Include the suite with `name`
-xname Exclude the suite with `name`
-v Increase verbosity (show suite names)
-q Only report tests that had an error
-Q Quit as soon as a test fails
-t Display results in tap format
-l Print suite names
-r[filename] Write summary file (to the optional filename)
Furthermore, running `make unit-tests` runs the binary along with all
the other unit tests we have.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The test driver in "unit-test.c" is responsible for setting up our unit
tests and eventually running them. As such, it is also responsible for
parsing the command line arguments.
The clar unit testing framework provides function `clar_test()` that
parses command line arguments and then executes the tests for us. In
theory that would already be sufficient. We have the special requirement
to always generate TAP-formatted output though, so we'd have to always
pass the "-t" argument to clar. Furthermore, some of the options exposed
by clar are ineffective when "-t" is used, but they would still be shown
when the user passes the "-h" parameter to have the clar show its usage.
Implement our own option handling instead of using the one provided by
clar, which gives us greater flexibility in how exactly we set things
up.
We would ideally not use any "normal" code of ours for this such that
the unit testing framework doesn't depend on it working correctly. But
it is somewhat dubious whether we really want to reimplement all of the
option parsing. So for now, let's be pragmatic and reuse it until we
find a good reason in the future why we'd really want to avoid it.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
