dbengine: a googletest suite for the engine - #23972

…h the contract for no engine at all

The suite is one executable that links the engine through the dbengine target,
so it inherits the usage requirements that target states rather than restating
them. That is the link an embedder outside this tree would make, which is why the
binary doubles as a standing check that the engine still links without the
daemon: it carries no daemon symbol, and nothing but the public headers is
reachable from the API tests.

Nothing in the build can enforce that last part - libnetdata exports the whole
src tree as a public include directory, so a private header would compile from
here without complaint - so check-public-includes.sh enforces it instead, and
files that test internals are exempt by name.

main() is the suite's own rather than googletest's, because two things have to be
settled before the first test and neither can be undone afterwards. The libuv
pool is sized at its first use and never resized, and the engine throttles itself
against the size it is told rather than the size there is; if those disagree the
process does not fail, it hangs. One constant feeds both the environment and the
configuration. The log limits are lifted because a failing test is read from the
engine's diagnostics.

The first cases are the contract for an embedder that never made an engine: every
getter answers as an engine with nothing in it, and every verb does nothing.
These duplicate what the existing suite already checks, and are written anyway:
they are the surface most likely to be touched by embedding work, and each is now
a named case that fails on its own instead of a line in a summed error count.
Two calls stay out - a tier init is fatal without an engine, by contract, and a
fatal ends the process, and a shutdown without an engine has nothing to observe.

…nitizer

A workflow of its own rather than a line in tests.yml. That job installs the
whole agent to run the in-binary suite, which is the right shape for what it runs
and the wrong shape for a target built by name in minutes, and it would not build
this one at all. Its path filter also covers only C sources and headers, so a
change confined to C++ triggers nothing there today.

The sanitizer leg configures with the project's own option rather than adding the
flags by hand. The option defines FSANITIZE_ADDRESS as well, which is what makes
libnetdata's pooled allocators fall back to malloc and free; with the flags alone
the pools stay intact and the sanitizer cannot see a use-after-free inside any of
them, so the leg would pass while checking very little.

Leak detection stays off. The engine keeps a great deal of process-lifetime state
on purpose and frees none of it at exit, so a leak check at exit reports residue
rather than findings. The contract worth having is checked directly instead:
destroying an engine returns the metrics still referenced, and the lifecycle
cases assert that it is none.

Both legs assert how many cases reported a result. A googletest binary whose
filter matches nothing exits zero having run nothing, so an exit code on its own
cannot tell a passing suite from one that never ran - which is the same way the
existing PGD test mode reports success while testing nothing.

…ses of their own

An engine is made, holds tiers of its own, is stopped and destroyed, and refuses
the calls the contract says it refuses. Each case brings its own engine up on its
own scratch directory and takes it back down, so none of them depends on another
having run, and a failure points at one verb rather than at a total.

The four refusals dbengine_tier_init() documents are each a case: a tier that is
already up, a tier that came up and exited, a tier on a stopped engine, and a
path with no room for the file names the engine appends to it. So is the file
descriptor budget, which is checked by giving the engine a budget of one - below
any tier's reservation, so the first tier is already too many. The size of that
reservation is not on the public surface, so the case is written not to need it.

Two tiers writing one directory corrupt it and nothing refuses the second, so
every case that brings a tier up takes a directory of its own from mkdtemp and
removes it afterwards.

Destroying an engine reports the registry metrics still referenced, and every
case asserts that it is none. That is the leak check this suite relies on: the
engine keeps process-lifetime state it never frees, so a leak detector at exit
reports residue rather than findings, while this number is exact and names the
thing it counts.

The configuration cases pin the defaults field by field rather than by comparing
the structures, because the padding between members is not part of the contract
and a drifted field should name itself.

… their own

Points go in through the public collect verbs and come back through the public
query verbs, and every field of what comes back is compared, anomaly_count
included - the field the existing comparison helper for storage points leaves
out, so nothing has ever checked it.

Two properties of the engine are written down here because getting them wrong is
how they were found. A point's start_time_s is the time of the point before it,
not its own, so a metric collected every second gives points whose start is their
end minus one. And tier 0 keeps values in the engine's own number format rather
than as doubles, so a stored 30 comes back as 30.000000000000004; values are
compared as doubles, which allows for the representation and still catches a
wrong number.

Each case takes an engine, a tier and a directory of its own, and a uuid nobody
else uses, so none of them can be made to pass or fail by another having run.

One case is disabled rather than absent. Deleting a metric's retention and then
destroying the engine reaches fatal() inside the uuid map, and fatal() ends the
process, so an enabled case would cost every result after it; the same sequence
also still reports the retention it just deleted. That is the engine's to fix and
not this change's work, so the reproducer stays, disabled, and is reported as
disabled on every run.

… ever had

They are pure functions over a buffer - no engine, no tier, no cache - and
nothing has ever exercised them. The cases cover what a codec must guarantee to
the extent writer: a payload survives the round trip, a repetitive one actually
gets smaller, the bound is never below the input a caller compresses in place,
and a codec byte read off disk that names an algorithm this build does not have
is refused rather than trusted.

The central contract is that compressing reports 0 to mean the payload is not
compressed, in two situations: the algorithm is the one that compresses nothing,
or a real codec produced something no smaller than the input, in which case it
leaves the payload exactly as it was. So a codec never reports a size that is not
an improvement, and the caller stores what it already has. The incompressible
case asserts that pair rather than assuming a codec always shrinks its input, and
the algorithm that compresses nothing is checked to leave the buffer untouched.

This is the first file here that reaches past the engine's public headers, so it
is named for that and the include check exempts it by name. The private headers
have no extern "C" of their own, except the one that wraps the rest, which is why
it is included first and the compression header is wrapped explicitly.

… asking for

The cache's existing test carries a list of twelve behaviours nobody wrote cases
for. These are those twelve: a clean page added and added again, a page released,
a hot page added and added again, a hot page turned dirty both with and without
another reference to it, dirty pages handed to the saver once enough of them
exist, the three ways of finding a page, eviction when the cache is full, and
hot pages saved when the cache is destroyed.

The cache needs no engine for any of it, so none of these cases brings one up,
and each takes a cache of its own with a size chosen for what it measures: small
enough to fill for eviction, large enough elsewhere that eviction never disturbs
what is being counted.

Three properties are written down because getting them wrong is how they were
found. The save callback must not call back into the cache for the pages it was
handed - the cache holds its own locks while calling it, and re-entering hangs
it, which is why the cache's own test callback does nothing. hot2dirty is a gauge
of pages in the middle of the move rather than a count of moves made, so it reads
zero once the move is done and the page itself has to be asked what it became.
And searching for the last page is relative to the time asked for, the newest
page at or before it, not the newest page there is.

…r own

Page descriptors are what a collector appends to and what an extent is built
from. The cases here are new rather than the ones already in the tree, which are
left exactly as they are: a page is empty when made, fills slot by slot, holds at
least the capacity it was asked for, gives its points back through a cursor and
nothing past the end, and survives being written into an extent and read back -
which is the trip every point takes to disk and home again.

The allocator layer they use is process-wide and keeps whatever the first caller
gave it, so the fixture initialises it from the same configuration the rest of
this binary uses. That makes these cases indifferent to whether an engine ran
before them, and one case pins the first-caller-wins rule itself so a change to
it cannot pass unnoticed.

The registry cases cover what it is for: finding a metric again, refusing to make
a second one for the same metric, keeping the same metric in two tiers apart,
remembering a retention, widening it when older or newer data turns up, refusing
to narrow it, and forgetting it when asked. They are separate cases because the
existing registry test asserts with fatal() for its whole second half, so the
first failure there ends the process and everything after it goes unrun.

A registry section is not a number but the address of the tier the metric belongs
to, which the registry follows to the engine's cache - so the fixture builds a
tier with its locks initialised and an engine with no event loop, rather than
passing a number that would be dereferenced.

The single-writer guard has no case: it exists only in internal-checks builds,
and a case that disappears with a build flag would quietly cover less in CI than
it does locally.

…, and stops asking for a page it cannot have

A page of a single slot is rejected for every page type, and the rejection is an
internal_fatal, so it ends the process and only in a build that has internal
checks turned on. One of the page descriptor cases asked for exactly that, which
passed everywhere it was run and would have died on any developer's build.

The sanitizer job now defines NETDATA_INTERNAL_CHECKS, as the older unit test job
already does. Without it the suite runs in CI under weaker checks than the build
it is written against, so a case that trips one of them passes CI and fails on a
desk - which is precisely what happened here.

…target that runs it

The option was the opt-in and the target was excluded from the default build as
well, so turning the option on still left the binary to be asked for by name. A
test that has to be remembered is a test that rots. The option alone now decides:
it is off by default, so an ordinary build has no such target at all, and when it
is on the suite builds with the rest. It still carries no install rule, so no
package can contain it whatever the option says.

Running it is a target of its own rather than a test registration, because this
tree has no CTest anywhere and adding one here would imply a harness that does
not exist. It runs on a terminal so the cases report as they go instead of
arriving in one block at the end, and a failing case fails the build.

An independent review mutated the engine under the suite and found cases that
stayed green with the behaviour they name completely broken.

The tier's metric count was sampled after the metric was created, so it already
included it, and the comparison only asked whether a counter had gone backwards
- it passed with the counter stubbed to zero. It is sampled before the create
now and the count must be exactly one higher.

The page footprint case is named for growth and asserted only that the footprint
had not shrunk, so it passed with the footprint stubbed to a constant. It asserts
growth.

The query-past-the-data case iterated the points it got back and asserted each
was a gap, but there are none, so the loop never ran and nothing was checked. It
asserts the emptiness its name claims, and keeps the gap check for the day that
changes.

All three were then mutated in the other direction to confirm they now fail.

Alongside: the compression cases refuse to run in a build with no codec rather
than looping over an empty list and reporting success; the algorithm check walks
every byte instead of starting past the last named one, so a codec this build
lacks is checked too; the page round trip guards its decompress the way its
siblings do, since decompressing something that was not compressed traps; a
scratch directory that could not be made is caught where it is made, instead of
becoming an empty path that ends the process; the two over-long paths are
described as the one check they meet rather than two; and the case that runs two
engines says why it must never be given a tier.

… can

The CI step that checks the suite links without the daemon looked at undefined
symbols. Undefined daemon symbols cannot reach that step: they would have failed
the link in the step before it. Daemon code that did get linked in would be
defined, not undefined, so the grep could never match and the step was
decoration. It looks at defined symbols now, which is the case worth catching -
the engine growing a dependency on its embedder, or daemon sources being added
to the target to make something compile.

The include check listed the private headers and rejected those, so anything it
had not thought of passed, and four ways through it were demonstrated: a path
containing the public prefix but walking out of it with dot-dot, a header named
for the internal exemption included from an API test, an include through a macro
the parser skipped, and a file extension the glob did not cover. It also never
looked for daemon headers at all, which is half of what it claims to enforce.

It is an allowlist now. A quoted include must be the suite's own shared header or
sit under the public header prefix; anything else fails, including an include the
script cannot parse, since not being able to read one is how it was defeated. All
four routes and both daemon-header cases were re-tested against it and are
refused, while legitimate files pass.

It also runs as part of building the suite rather than only in CI, so the
developer who would add such an include is the one who hears about it, and its
unreachable no-headers-found branch is gone with the header list that needed it.

… it shows

Its comment claimed the delete also leaves the retention reported. That came from
a variant written while the bug was being narrowed, which held the metric handle
across the delete - and a held handle means the metric is not deleted, so its
retention legitimately remains. The committed case releases first and its
retention assertions pass; what remains is the page that outlives its metric.

…aker one nearby

A second independent review found cases that held while the thing they describe
was broken.

The dirty-page case is named for the cache saving once enough pages are dirty,
and asked its question after calling the flush itself - so it would have passed
with automatic flushing entirely gone. It asserts before asking, and only then
drains. Measured: all eight pages are already saved by that point, so the
contract does hold; what was missing was the ability to notice if it stopped.

A hot page that loses its last reference was checked for no longer being hot,
which any state change would satisfy. It must become dirty, so that is what it
says now.

The tier's sample count was asserted to be more than none, which a counter stuck
at any positive number satisfies. Two points one second apart cover one second
and the tier counts the span rather than the points, so the expected number is
exactly one.

The directory check only ever asked about directories with no datafiles, both
expected to answer no - it would have passed with the function stubbed to always
say no. It now brings a tier up and asserts the directory reports them, before
and after the engine goes.

Destroying a page cache returns false when pages are still referenced and it had
to stay allocated. The fixture discarded that, so a case that forgot to release a
page would leak quietly and still pass. Two lookups that expect to find nothing
now release what they find if they ever do, so that a surprise there is one
failure rather than a failure and an undestroyable cache.

The null-engine memory case was named for an engine behaviour but asserted a
libnetdata helper's own guard, which returns zero for an empty slot by
construction and cannot fail. It is named for what it pins - that the daemon may
read every slot without checking which are populated - and says where the engine
side is actually covered.

The include check now walks subdirectories, since a file added in one would have
been skipped without a word.

… when it cannot clean up

The two test files each carried their own copy, both of them hardcoding /tmp,
both removing one flat level, and both ignoring what unlink and rmdir returned.
The plan said the root should come from the environment; the code did not, and
three reviewers did not catch it either.

There is one copy now, in the shared header.

It takes TMPDIR when the environment names one. A runner or a sandbox that
redirects it usually cannot write to /tmp at all, and a suite that ignores it
fails there for a reason that looks like the engine's fault.

Cleanup descends instead of assuming one flat level, and skips only dot and
dot-dot rather than every dotfile. A tier writes two plain files today, but a
cleanup that quietly cannot cope with anything else is one that will quietly stop
working.

And it reports. Removal that fails now fails the test instead of leaving a
directory behind that nobody hears about - which is the same silent-success
problem the review has just finished removing from the cases themselves.

All three were exercised: a run under a TMPDIR of its own leaves nothing there
and nothing in /tmp; the same run against a read-only TMPDIR fails the case
rather than ending the process; removal takes a tree with a subdirectory and a
dotfile in it; and removal of something it cannot delete returns false.

…ld not do, and the CI gate lost its own diagnostic

The case asserting that a repetitive payload compresses accepted a codec that
declined to compress it: declining returns 0, and 0 is smaller than the payload,
so the one scenario the case exists to catch - a codec silently doing nothing -
passed it. It now requires the payload to have been compressed before comparing
sizes, the way its sibling already did.

The registry fixture discarded what destroying it reported. It returns the
metrics still referenced and stays allocated when there are any, so a case that
forgot a release leaked quietly and still passed. Same shape as the page cache
fixture, which was fixed earlier; both are checked now.

The null-engine memory case asserted four values that cannot differ: those
readers return zero for an empty slot by construction. It is folded into the case
that does assert engine behaviour - that a null engine populates none of its own
slots - keeping the sweep over every slot for what it is worth, which is that
the daemon may read them all without crashing, and asserting nothing about
numbers that cannot change.

The query-past-the-data case described itself as tolerating a future engine that
returned gaps instead of nothing. It could not: the reader stops after 64 points
and fails, so such an engine would fail long before the tolerance applied.
Promising something the code cannot deliver is worse than not promising it.

The page footprint case appended a fixed number of points and asserted growth,
which for this page type only happens when the encoder takes another buffer -
making it a bet on how many encoded values fit in one. It appends until the
footprint moves, bounded by the page's capacity, and fails if it never does.

In CI, a failing or stalled binary exited the step before the case-count check
could print its diagnostic - in exactly the situation that check exists for. The
status is captured, the count is reported either way, and the run's own status
then decides the step.