Skip to content

Test improvements#4269

Merged
ethomson merged 9 commits intolibgit2:masterfrom
pks-t:pks/tests
Jun 14, 2017
Merged

Test improvements#4269
ethomson merged 9 commits intolibgit2:masterfrom
pks-t:pks/tests

Conversation

@pks-t
Copy link
Copy Markdown
Member

@pks-t pks-t commented Jun 13, 2017

This adds additional tests for the ODB, options initialization and fixes an issue with the initialization tests always being skipped on non-MSVC platforms due to "-D_DEBUG" not being defined.

pks-t added 9 commits June 13, 2017 11:11
In commit 9f75a9c (Turning on runtime checks when building debug under
MSVC., 2012-03-30), we introduced a comment "Precompiled headers", which
actually refers to no related commands. Seeing that the comment never
had anything to refer to, we can simply remove it here.
In our code base, we have some occasions where we use the "_DEBUG"
preprocessor macro to enable additional code which should not be part of
release builds. While we define this flag on MSVC platforms, it is
guarded by the conditional `WIN32 AND NOT CYGWIN` on other platforms
since 19be3f9 (Improve MSVC compiler, linker flags, 2013-02-13). While
this condition can be fulfilled by the MSVC platform, it is never
encountered due to being part of the `ELSE` part of `IF (MSVC)`.

The intention of the conditional was most likely to avoid the
preprocessor macro on Cygwin platforms, but to include it on everthing
else. As such, the correct condition here would be `IF (NOT CYGWIN)`
instead. But digging a bit further, the condition is only ever used in
two places:

1. To skip the test in "core::structinit", which should also work on
   Cygwin.
2. In "src/win32/git2.rc", where it is used to set additional file
   flags. As this file is included in MSVC builds only, it cannot cause
   any harm to set "_DEBUG" on Cygwin here.

As such, we can simply drop the conditional and always set "-D_DEBUG" on
all platforms.
While our debug builds on MSVC platforms already tune the code optimizer
to aid debugging code, all the other platforms still use the default
optimization level. This makes it hard for developers on these platforms
to actually debug code while maintaining his sanity due to optimizations
like inlined code, elided variables etc.

To help this common use case, we can simply follow the MSVC example and
turn off code optimization with "-O0" for debug builds. While it would
be preferable to instead use "-Og" supported by more modern compilers,
we cannot guarantee that this level is available on all supported
platforms.
A newly added test uses the `git_repository_new` function without the
corresponding header file being included. While this works due to the
compiler deducing the correct function signature, we should obviously
just include the function's declaration file.
Initialization of the `git_proxy_options` structure is never tested
anywhere. Include it in our usual initialization test in
"core::structinit::compare".
In order to be able to test the ODB prefix functions, we need to be able
to detect ambiguous prefixes in case multiple objects with the same
prefix exist in the fake ODB. Extend `search_object` to detect ambiguous
queries and have callers return its error code instead of always
returning `GIT_ENOTFOUND`.
The `search_object` function takes the OID length as one of its
parameters, where its maximum length is `GIT_OID_HEXSZ`. The `exists`
function of the fake backend used `GIT_OID_RAWSZ` though, leading to
only the first half of the OID being used when finding the correct
object.
The fake backend currently implements all reading functions except for
the `exists_prefix` one. Implement it to enable further testing of the
ODB layer.
Introduce a new test suite "odb::backend::simple", which utilizes the
fake backend to exercise the ODB abstraction layer. While such tests
already exist for the case where multiple backends are put together, no
direct testing for functionality with a single backend exist yet.
@ethomson ethomson merged commit 953427b into libgit2:master Jun 14, 2017
@pks-t pks-t deleted the pks/tests branch September 15, 2017 06:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants