CMake(Visual C) support for js/doc-unit-tests
#1579
Conversation
In our current testing environment, we spend a significant amount of effort crafting end-to-end tests for error conditions that could easily be captured by unit tests (or we simply forgo some hard-to-setup and rare error conditions). Describe what we hope to accomplish by implementing unit tests, and explain some open questions and milestones. Discuss desired features for test frameworks/harnesses, and provide a preliminary comparison of several different frameworks. Co-authored-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Josh Steadmon <steadmon@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This patch contains an implementation for writing unit tests with TAP
output. Each test is a function that contains one or more checks. The
test is run with the TEST() macro and if any of the checks fail then the
test will fail. A complete program that tests STRBUF_INIT would look
like
#include "test-lib.h"
#include "strbuf.h"
static void t_static_init(void)
{
struct strbuf buf = STRBUF_INIT;
check_uint(buf.len, ==, 0);
check_uint(buf.alloc, ==, 0);
if (check(buf.buf == strbuf_slopbuf))
return; /* avoid SIGSEV */
check_char(buf.buf[0], ==, '\0');
}
int main(void)
{
TEST(t_static_init(), "static initialization works);
return test_done();
}
The output of this program would be
ok 1 - static initialization works
1..1
If any of the checks in a test fail then they print a diagnostic message
to aid debugging and the test will be reported as failing. For example a
failing integer check would look like
# check "x >= 3" failed at my-test.c:102
# left: 2
# right: 3
not ok 1 - x is greater than or equal to three
There are a number of check functions implemented so far. check() checks
a boolean condition, check_int(), check_uint() and check_char() take two
values to compare and a comparison operator. check_str() will check if
two strings are equal. Custom checks are simple to implement as shown in
the comments above test_assert() in test-lib.h.
Tests can be skipped with test_skip() which can be supplied with a
reason for skipping which it will print. Tests can print diagnostic
messages with test_msg(). Checks that are known to fail can be wrapped
in TEST_TODO().
There are a couple of example test programs included in this
patch. t-basic.c implements some self-tests and demonstrates the
diagnostic output for failing test. The output of this program is
checked by t0080-unit-test-output.sh. t-strbuf.c shows some example
unit tests for strbuf.c
The unit tests can be built with "make unit-tests" (this works but the
Makefile changes need some further work). Once they have been built they
can be run manually (e.g t/unit-tests/t-strbuf) or with prove.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Josh Steadmon <steadmon@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Run unit tests in both Cirrus and GitHub CI. For sharded CI instances (currently just Windows on GitHub), run only on the first shard. This is OK while we have only a single unit test executable, but we may wish to distribute tests more evenly when we add new unit tests in the future. We may also want to add more status output in our unit test framework, so that we can do similar post-processing as in ci/lib.sh:handle_failed_tests(). Signed-off-by: Josh Steadmon <steadmon@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
dscho
force-pushed
the
doc-unit-tests-and-cmake
branch
from
20d8f2d
to
fb03f5a
Compare
A new, better way to run unit tests was just added to Git. This adds support for building those unit tests via CMake. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
When building the unit tests via CMake, the `.pdb` files are built. Those are, essentially, files containing the debug information separately from the executables. Let's not confuse them with the executables we actually want to run. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
|
/submit |
|
Submitted as pull.1579.git.1693462532.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
This branch is now known as |
|
This patch series was integrated into seen via git@cba8fd9. |
|
This patch series was integrated into seen via git@74ba33e. |
|
There was a status update in the "New Topics" section about the branch Update the base topic to work with CMake builds. source: <pull.1579.git.1693462532.gitgitgadget@gmail.com> |
|
This patch series was integrated into seen via git@a58eb82. |
|
This patch series was integrated into seen via git@d10c4d7. |
|
There was a status update in the "Cooking" section about the branch Update the base topic to work with CMake builds. source: <pull.1579.git.1693462532.gitgitgadget@gmail.com> |
|
This patch series was integrated into seen via git@297ccdf. |
|
This patch series was integrated into seen via git@264b5b8. |
|
This patch series was integrated into seen via git@c18acc7. |
|
This patch series was integrated into seen via git@d772625. |
|
This patch series was integrated into seen via git@60d3dc0. |
|
This patch series was integrated into seen via git@4f73685. |
![gitgitgadget[bot]](https://avatars.githubusercontent.com/in/12836?s=60&v=4){kind=small}
| @@ -21,6 +21,42 @@ static struct { | |||
| .result = RESULT_NONE, | |||
There was a problem hiding this comment.
On the Git mailing list, Phillip Wood wrote (reply to this):
Hi Johannes
On 31/08/2023 07:15, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> > Visual C interpolates `__FILE__` with the absolute _Windows_ path of
> the source file. GCC interpolates it with the relative path, and the
> tests even verify that.
Oh, that's a pain
> So let's make sure that the unit tests only emit such paths.
Makes sense
> +#ifndef _MSC_VER
> +#define make_relative(location) location
> +#else
> +/*
> + * Visual C interpolates the absolute Windows path for `__FILE__`,
> + * but we want to see relative paths, as verified by t0080.
> + */
> +#include "strbuf.h"
> +#include "dir.h"
> +
> +static const char *make_relative(const char *location)
> +{
> + static const char *prefix;
> + static size_t prefix_len;
> + static struct strbuf buf = STRBUF_INIT;
So far test-lib.c avoids using things like struct strbuf that it will be used to test. In this instance we're only using it on one particular compiler so it may not matter so much. We could avoid it but I'm not sure it is worth the extra complexity. One thing I noted in this patch is that prefix is leaked but I'm not sure if you run any leak checkers on the msvc build.
static const char *make_relative(const char *location)
{
static char prefix[] = __FILE__
static size_t *prefix_len = (size_t)-1;
static char buf[PATH_MAX];
if (prefix == (size_t)-1) {
const char *path = "\\t\unit-tests\\test-lib.c";
size_t path_len = strlen(path);
prefix_len = strlen(prefix);
if (prefix_len < path_len) ||
memcmp(prefix + prefix_len - path_len, path, path_len)
die(...);
prefix_len -= path_len - 1; /* keep trailing '\\' */
prefix[prefix_len] = '\0';
}
/* Does it not start with the expected prefix? */
if (fspathncmp(location, prefix, prefix_len))
return location;
if (strlen(location) - prefix_len > sizeof(buf) - 1)
die(...)
/* +1 to copy NUL terminator */
memcpy(buf, location + prefix_len, strlen(location) - prefix_len + 1);
convert_slashes(buf);
return buf;
}
Best Wishes
PhillipThere was a problem hiding this comment.
On the Git mailing list, Johannes Schindelin wrote (reply to this):
Hi Phillip,
On Mon, 11 Sep 2023, Phillip Wood wrote:
> On 31/08/2023 07:15, Johannes Schindelin via GitGitGadget wrote:
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > Visual C interpolates `__FILE__` with the absolute _Windows_ path of
> > the source file. GCC interpolates it with the relative path, and the
> > tests even verify that.
>
> Oh, that's a pain
>
> > So let's make sure that the unit tests only emit such paths.
>
> Makes sense
>
> > +#ifndef _MSC_VER
> > +#define make_relative(location) location
> > +#else
> > +/*
> > + * Visual C interpolates the absolute Windows path for `__FILE__`,
> > + * but we want to see relative paths, as verified by t0080.
> > + */
> > +#include "strbuf.h"
> > +#include "dir.h"
> > +
> > +static const char *make_relative(const char *location)
> > +{
> > + static const char *prefix;
> > + static size_t prefix_len;
> > + static struct strbuf buf = STRBUF_INIT;
>
> So far test-lib.c avoids using things like struct strbuf that it will be used
> to test. In this instance we're only using it on one particular compiler so it
> may not matter so much. We could avoid it but I'm not sure it is worth the
> extra complexity. One thing I noted in this patch is that prefix is leaked but
> I'm not sure if you run any leak checkers on the msvc build.
I changed the code not to use a strbuf, and I'm now working exclusively on
static buffers instead of `malloc()`ing anything.
Thank you,
Johannes
|
User |
| @@ -965,6 +965,24 @@ target_link_libraries(test-fake-ssh common-main) | |||
| parse_makefile_for_sources(test-reftable_SOURCES "REFTABLE_TEST_OBJS") | |||
There was a problem hiding this comment.
On the Git mailing list, Phillip Wood wrote (reply to this):
Hi Johannes
On 31/08/2023 07:15, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> > A new, better way to run unit tests was just added to Git. This adds
> support for building those unit tests via CMake.
This patch builds the unit tests but does not add them to the list of tests run by CTest - how are the tests typically run on the CMake build?
Best Wishes
Phillip
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> contrib/buildsystems/CMakeLists.txt | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
> > diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
> index 2f6e0197ffa..45016213358 100644
> --- a/contrib/buildsystems/CMakeLists.txt
> +++ b/contrib/buildsystems/CMakeLists.txt
> @@ -965,6 +965,24 @@ target_link_libraries(test-fake-ssh common-main)
> parse_makefile_for_sources(test-reftable_SOURCES "REFTABLE_TEST_OBJS")
> list(TRANSFORM test-reftable_SOURCES PREPEND "${CMAKE_SOURCE_DIR}/")
> > +#unit-tests
> +add_library(unit-test-lib OBJECT ${CMAKE_SOURCE_DIR}/t/unit-tests/test-lib.c)
> +
> +parse_makefile_for_scripts(unit_test_PROGRAMS "UNIT_TEST_PROGRAMS" "")
> +foreach(unit_test ${unit_test_PROGRAMS})
> + add_executable("${unit_test}" "${CMAKE_SOURCE_DIR}/t/unit-tests/${unit_test}.c")
> + target_link_libraries("${unit_test}" unit-test-lib common-main)
> + set_target_properties("${unit_test}"
> + PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/t/unit-tests)
> + if(MSVC)
> + set_target_properties("${unit_test}"
> + PROPERTIES RUNTIME_OUTPUT_DIRECTORY_DEBUG ${CMAKE_BINARY_DIR}/t/unit-tests)
> + set_target_properties("${unit_test}"
> + PROPERTIES RUNTIME_OUTPUT_DIRECTORY_RELEASE ${CMAKE_BINARY_DIR}/t/unit-tests)
> + endif()
> + list(APPEND PROGRAMS_BUILT "${unit_test}")
> +endforeach()
> +
> #test-tool
> parse_makefile_for_sources(test-tool_SOURCES "TEST_BUILTINS_OBJS")
> There was a problem hiding this comment.
On the Git mailing list, Johannes Schindelin wrote (reply to this):
Hi Phillip,
On Mon, 11 Sep 2023, Phillip Wood wrote:
> On 31/08/2023 07:15, Johannes Schindelin via GitGitGadget wrote:
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > A new, better way to run unit tests was just added to Git. This adds
> > support for building those unit tests via CMake.
>
> This patch builds the unit tests but does not add them to the list of tests
> run by CTest - how are the tests typically run on the CMake build?
You're right, I missed that the unit tests are run as part not of t0080,
but as a separate target in `t/Makefile`.
I've added a couple of patches to clean up the CTest part and then run the
unit test (t-strbuf, and whatever is added to the `UNIT_TEST_PROGRAMS`
variable in `Makefile`).
Thank you,
Johannes
|
This patch series was integrated into seen via git@f448e0d. |
|
This patch series was integrated into seen via git@9a8475e. |
|
There was a status update in the "Cooking" section about the branch Update the base topic to work with CMake builds. source: <pull.1579.git.1693462532.gitgitgadget@gmail.com> |
|
There was a status update in the "Cooking" section about the branch Update the base topic to work with CMake builds. source: <pull.1579.git.1693462532.gitgitgadget@gmail.com> |
|
There was a status update in the "Cooking" section about the branch Update the base topic to work with CMake builds. Will merge to 'next'? source: <pull.1579.v3.git.1695640836.gitgitgadget@gmail.com> |
|
This patch series was integrated into seen via git@46657c0. |
|
This patch series was integrated into seen via git@e9596bc. |
|
This patch series was integrated into seen via git@8356f2a. |
|
There was a status update in the "Cooking" section about the branch Update the base topic to work with CMake builds. Will merge to 'next'? source: <pull.1579.v3.git.1695640836.gitgitgadget@gmail.com> |
dscho
force-pushed
the
js/doc-unit-tests
branch
from
7129a07
to
d8f416b
Compare
|
This patch series was integrated into seen via git@7202ee7. |
|
This patch series was integrated into next via git@b4503c9. |
|
This patch series was integrated into seen via git@fff81f9. |
|
There was a status update in the "Cooking" section about the branch Update the base topic to work with CMake builds. Will cook in 'next'. source: <pull.1579.v3.git.1695640836.gitgitgadget@gmail.com> |
|
This patch series was integrated into seen via git@467de75. |
|
This patch series was integrated into seen via git@248dc28. |
|
This patch series was integrated into seen via git@1a1984f. |
|
There was a status update in the "Cooking" section about the branch Update the base topic to work with CMake builds. Will cook in 'next'. source: <pull.1579.v3.git.1695640836.gitgitgadget@gmail.com> |
|
This patch series was integrated into seen via git@01f8a7d. |
|
This patch series was integrated into seen via git@4f3c080. |
|
There was a status update in the "Cooking" section about the branch Update the base topic to work with CMake builds. Will cook in 'next'. source: <pull.1579.v3.git.1695640836.gitgitgadget@gmail.com> |
|
This patch series was integrated into seen via git@d9c9f09. |
|
There was a status update in the "Cooking" section about the branch Update the base topic to work with CMake builds. Will cook in 'next'. source: <pull.1579.v3.git.1695640836.gitgitgadget@gmail.com> |
|
There was a status update in the "Cooking" section about the branch Update the base topic to work with CMake builds. Will merge to 'master'. source: <pull.1579.v3.git.1695640836.gitgitgadget@gmail.com> |
|
This patch series was integrated into seen via git@f5b067a. |
|
There was a status update in the "Cooking" section about the branch Update the base topic to work with CMake builds. Will merge to 'master'. source: <pull.1579.v3.git.1695640836.gitgitgadget@gmail.com> |
|
This patch series was integrated into seen via git@712177e. |
|
This patch series was integrated into master via git@712177e. |
|
This patch series was integrated into next via git@712177e. |
|
Closed via 712177e. |
The recent patch series that adds proper unit testing to Git requires a couple of add-on patches to make it work with the CMake build on Windows (Visual C). This patch series aims to provide that support.
This patch series is based on
js/doc-unit-tests.Changes since v2:
Changes since v1:
test-lib.cnow avoids using astrbuf.cc: Phillip Wood phillip.wood123@gmail.com