For consistent ordering in later scripts. The previous
-F=min(enumerate()) trick mostly worked, but would get messed up by
running things in parallel (-j).
I've already confused myself a couple times looking at script output,
which is never a good sign.
This reworks -P/--powerloss to be more consistent with other flexible
flags (-D/--define, -S/--probe, etc):
- Tweaks -P/--powerloss to accept multiple flags (-Pnone -Plinear)
instead of a comma-separated list (-Pnone,linear)
- Adopts an expr-like grammar similar to -Dx='range(3)', -Sx=123shz, etc
(see below)
- Generalizes run_powerloss_linear and run_powerloss_log to accept
start/stop/step conditions, allowing for range and logrange exprs
with minimal work
---
The new expr-like grammar follows what's worked well for -D/--define,
-S/--probe, etc, in which parens can be used to parameterize some of the
more complex scenarios. This makes the -P/--powerloss grammar more
consistent, less ad-hoc, easier to parse, while also providing
flexibility for future powerloss exprs.
As an example, bounded range/logrange variants of linear/log were easy
to add without each needing their own little syntax:
- none -> none - Run with no powerlosses
- linear -> linear - Run with linearly-decreasing powerlosses
- log -> log - Run with exponentially-decreasing pls
- n -> permute(n) - Run all permutations of n powerlosses
- exhaustive -> exhaustive - Run all powerloss permutations
- {1,2,3} -> list(1,2,3) - Run explicit list of powerlosses
- added range(a,b,s) - Run explicit range of powerlosses
- added logrange(a,b,s) - Run explicit range of 2^n powerlosses
- :1248g1 -> :1248g1 - Run custom leb128-encoded set of pls
Note we still keep :-prefixed leb128-encoded powerlosses as is. This is
enough of its own syntax that trying to map it to an expr doesn't really
make sense. And is humorously compatible with most future grammars.
- -S/--probe - Specify a probe to sample.
- -x/--probe-step - Sample probes every n steps.
- --probe-runfreq - Sample probes at this frequency in hz.
- -X/--probe-simfreq - Sample probes at this frequency in simulated hz.
Also:
- --trace-simfreq - Sample trace output at this frequency in
simulated hz.
These give finer grain control over which probes we measure during
benching, and how we measure them.
These also introduce several exciting bench features:
- -S/--probe provides the ability to easily filter which probes you're
interested in at runtime.
This should replace the growing use of MASK defines in the benches.
- -x/--probe-step makes it easy to relax sampling rate when the amount
of data overwhelms later scripts.
This should replace the growing use of STEP defines in the benches.
- The additional concept of simfreq, which allows perf-esque sampling in
simtime. This provides another option for intuitively relaxing probe
sampling rate without sacrificing reproducibility.
(runfreq depends on wall time, so good bye reproducibility, though may
still be useful in interactive contexts.)
Note -S/--probe and -x/--probe-step replace MASK/STEP defines, which
have already proved their usefulness, but required reimplementation in
every bench case. An obvious contender to move into the bench_runner!
---
Note note that -S/--probe also supports some simple sample expressions,
allowing flexible step/simfreq/runfreq at the per-probe level:
- -Swrite=100 - Sample probe "write" every 100 steps
- -Swrite=100rhz - Sample probe "write" 100 times a runtime second
- -Swrite=100shz - Sample probe "write" 100 times a simulated second
Though I wonder how long it will take before I forget this feature
exists.
Mainly to make space for some planned bench flags, while also preferring
"step" over "period" (for consistency), and "runfreq" over "freq" (to
differentiate from "simfreq" in the future).
In runners:
- -s/--step -> --step
- --trace-period -> --trace-step
- --trace-freq -> --trace-runfreq
In scripts:
- --record -> -e/--record
- --perf-period -> --perf-step
- --perf-freq -> --perf-runfreq
- --include -> -i/--include
---
One thing that makes this work is the new sys.argv regex trick, where we
try to predict what mode the script will run in by prematching known
mode-switch flags before handing things off to argparse.
Note:
- Hiding flags from argparse risks confusing help-text, so we include
all flags if we see -h/--help in sys.argv.
This doesn't work for the help-text printed if argparse errors, but we
can only do so much. Maybe argparse only showing relevant flags for
the given mode is ok?
- We use -[^-]*[hf].* for shortform flags, which should also match
multiple shortform flags in a single arg (-fhfhfh).
- This requires the conflict_handler='ignore' hack to work, but these
scripts already needed it anyways.
--no-internal has already proven useful for skipping internal tests for
refactoring, so it makes sense to add --no-reentrant/fuzz flags as well.
--no-fuzz seems particularly useful for when you want to skip the less
targeted fuzz tests:
- with fuzz tests: 634616/634616 passed, in 1239.90s
- with --no-fuzz: 85434/85434 passed, in 423.41s
I also added runtime variants to test/bench_runner and test/bench.py.
These may be useful to skip tests without needing to recompile the
runner.
---
Also tweaked -s/--step to filter permutations in any --list-* flags, for
consistency.
This adds an explicit:
internal = true
As an alternative to:
in = 'lfs3.c'
For marking tests/benches as internal without actually placing them in a
specific source file.
The internal flag and --no-internal have proven suprisingly useful for
running a subset of tests when refactoring, as internal tests break much
more frequently than the high-level API. However, placing all the
internal tests in lfs3.c _has_ put a big strain on compilation/link
times.
`internal = true` now lets you mark tests/benches as internal, without
the extra compile/link overhead. You don't get access to any internal
things, but the flag can still be useful for filtering.
---
The original motivation for this was in the test_fwrite_clip_* tests,
but they ended up using lfs3_bptr_size to check leaf sizes, so oh well.
At least it's a good flag to have around. (In theory these could be made
"fake internal" with a manual bitmask, but it doesn't seem worth the
potential maintenance headache for saving a bit of link time. Though
maybe in the future priorities will change.)
Also cleaned up the handling of None in test/bench config a bit. Now
None should be equivalent to missing config fields, at the cost of more
noise in the Python code. None vs missing always feels unusually clunky
in Python.
The main changes:
- Added TEST_DEFINES and BENCH_DEFINES to allow overriding the default
test/bench define header:
-DTEST_DEFINES=my_test_defines.h
Note these are VERY different from LFS_DEFINES upstream. They aren't a
typical header file, and are included multiple times with various
query macros.
It's hacky, but works surprisingly well.
Or maybe I'll just do anything to avoid having to write multiline
macros. Ugh, backslashes.
- Moved more logic into bench/test_defines.h, including everything
needed to integrate other filesystems out-of-tree.
This mostly meant moving all of the cfg initialization logic into its
own query macro (replacing the BENCH/TEST_CFG field macros).
But this also includes a bunch of small tweaks encountered while trying
to get external benchmarks running again.
The external benchmarks include several other filesystems (littlefs2,
SPIFFS, Yaffs2), and I'm hoping this injectable/queryable header thing
will do a good job at avoiding a maintenance headache. (At least a
better job than forking bench_runner.c, which was the previous
solution.)
By "simple" I mean any non-globbing suite/case ids.
Non-globbing suite/case ids can be filtered early in the test_runner.
But globbing ids require, surprise, globbing, which is currently handled
by test/bench.py.
---
This greatly speeds up valgrind testing of specific suites/cases,
otherwise things get bogged down during the initial --list-cases due to
the sheer number of test permutations we've accumulated.
Maybe we shouldn't be running the initial --list-cases under Valgrind,
but oh well. This mostly solves the problem without too many changes.
The value of simtime isn't actually the simtime value, but the
simulated throughput, which is easy enough for our csv.py script to
calculate (with a daintily placed max to avoid divide-by-zero).
Throughput has the benefit of being somewhat size-agnostic, making
cross-benchmark comparisons easier.
I guess it's technically possible to do something similar with
readed/progged/erased numbers, but conceptually that would be really
confusing...
---
Also renamed test/bench_time -> test/bench_runtime to hopefully prevent
confusion between the two time spaces.
- Delayed defines/permutations assignment until after generation. Just a
bit of code smell.
- Expanded all __eq__, __ne__, __lt__, __gt__, etc magic methods, just
to minimize surprises in the future.
This extends our ifdef/ifndef test attributes to support more
complicated logic expressions.
So far we haven't really needed this (ifdef/ifndef accepts an implicitly
anded list, which has covered everything so far), but I realized there's
a simple trick to make this work.
For example, in test.toml:
ifdef = 'A && !(B || C)'
Generated ifdef:
#if (defined(A) && !(defined(B) || defined(C)))
This doesn't require complex parsing or anything, just a simple regex:
s/[a-zA-Z_0-9]\+/defined(&)/g
Is using #if defined(A) everywhere instead of #ifdef A more expensive
for the compiler? Not sure. But it seems like we're heavily dominated by
the single-threaded link time, so I'm not sure we care.
Test/bench filters have proven to be mostly non-optional, protecting
against bad configuration that doesn't make any sense.
It's still valid to want to override test filters sometimes, but using a
more, uh, forceful verb probably makes sense here.
The shortform would conflict with -f/--fail, so no shortform flag for
this, but some argue --force should never have a shortform flag anyways.
Too much room for confusion, and potential flag conflicts in the future.
Note it already conflicted with -e/--error-* flags.
--exec is a rather technical flag anyways, and will probably be wrapped
in other ci/script scaffolding most of the time.
The --no-internal flag avoids building any internal tests/benches
(tests/benches with in="lfs3.c"), which can be useful for quickly
testing high-level things while refactoring. Refactors tend to break all
the internal tests, and it can be a real pain to update everything.
Note that --no-internal can be injected into the build with TESTCFLAGS:
TESTCFLAGS=--no-internal make test-runner -j \
&& ./scripts/test.py -j -b
For a curious data point, here's the current number of
internal/non-internal tests:
suites cases perms
total: 24 808 633968/776298
internal: 22 (91.7%) 532 (65.8%) 220316/310247 (34.8%)
non-internal: 2 ( 8.3%) 276 (34.2%) 413652/466051 (65.2%)
It's interesting to note that while internal tests have more test cases,
the non-internal tests generate a larger number of test permutations.
This is probably because internal tests tend to target specific corner
cases/known failure points, and don't invite much variants.
---
While --no-internal may be useful for high-level testing during a
refactor, I'm not sure it's a good idea to rely on it for _debugging_ a
refactor.
The whole point of internal testing is to catch low-level bugs early,
with as little unnecessary state as possible. Skipping these to debug
integration tests is a bit counterproductive!
Note this includes both the lfs3_config -> lfs3_cfg structs as well as
the LFS3_CONFIG -> LFS3_CFG include define:
- LFS3_CONFIG -> LFS3_CFG
- struct lfs3_config -> struct lfs3_cfg
- struct lfs3_file_config -> struct lfs3_file_cfg
- struct lfs3_*bd_config -> struct lfs3_*bd_cfg
- cfg -> cfg
We were already using cfg as the variable name everywhere. The fact that
these names were different was an inconsistency that should be fixed
since we're committing to an API break.
LFS3_CFG is already out-of-date from upstream, and there's plans for a
config rework, but I figured I'd go ahead and change it as well to lower
the chances it gets overlooked.
---
Note this does _not_ affect LFS3_TAG_CONFIG. Having the on-disk vs
driver-level config take slightly different names is not a bad thing.
Like test.py --gdb-script, being able to specify multiple header files
seems useful and is easy enough to add.
---
Note that the default is only used if no other header files are
specified, so this _replaces_ the default header file:
$ ./scripts/test.py --include=my_header.h
If you don't want to replace the default header file, you currently need
to specify it explicitly:
$ ./scripts/test.py \
--include=runners/test_runner.h \
--include=my_header.h
These just invoke the existing dbg*.py python scripts, but allow quick
references to variables in the debugginged process:
(gdb) dbgflags o file->b.o.flags
LFS3_O_RDWR 0x00000002 Open a file as read and write
LFS3_o_REG 0x10000000 Type = regular-file
LFS3_o_UNSYNC 0x01000000 File's metadata does not match disk
Quite neat and useful!
This works by injecting dbg.gdb.py via gdb -x, which includes the
necessary python hooks to add these commands to gdb. This can be
overridden/extended with test.py/bench.py's --gdb-script flag.
Currently limited to scripts that seem the most useful for process
internals:
- dbgerr - Decode littlefs error codes
- dbgflags - Decode littlefs flags
- dbgtag - Decode littlefs tags
This actually binds our custom write/writeln functions as methods to the
file object:
def writeln(self, s=''):
self.write(s)
self.write('\n')
f.writeln = writeln.__get__(f)
This doesn't really gain us anything, but is a bit more correct and may
be safer if other code messes with the file's internals.
As you might expect, this is the inverse of ifdef, and is useful for
supporting opt-out flags.
I don't think ifdef + ifndef is powerful enough to handle _all_
compile-time corner cases, but they at least provide convenient handling
for the most common flags. Worst case, tests/benches can always include
explicit #if/#ifdef/#ifndef statements in the code itself.
Reading Wikipedia:
> Later terminals added the ability to directly specify the "bright"
> colors with 90–97 and 100–107.
So if we want to stick to one pattern, we should probably go with
brightness as a separate modifier.
This shouldn't noticeably change any script, unless your terminal
interprets 90-97m colors differently from 1;30-37m, in which case things
should be more consistent now.
Why not, -e/--exec seems useful/general purpose enough to deserve a
shortform flag. Especially since much of our testing involves emulation.
The only risk of conflicts is with -e/--error-* in other scripts, but
the _whole point_ of test.py is to error on failure, so I don't think
this will be an issue.
Note that -E may be more useful for environment variables in the future.
I feel like -e/--exec was more common in other programs, but I've only
found sed -e and perl -e so far. Most programs stick to -c/--command
(bash, python) which would conflict with -c/--compile here.
This only failed if "-" was used as an argument (for stdin/stdout), so
the issue was pretty hard to spot.
openio is a heavily copy-pasted function, so it makes sense to just add
the import os to openio directly. Otherwise this mistake will likely
happen again in the future.
Now that I'm looking into some higher-level scripts, being able to merge
results without first renaming everything is useful.
This gives most scripts an implicit prefix for field fields, but _not_
by fields, allowing easy merging of results from different scripts:
$ ./scripts/stack.py lfs.ci -o-
function,stack_frame,stack_limit
lfs_alloc,288,1328
lfs_alloc_discard,8,8
lfs_alloc_findfree,16,32
...
At least now these have better support in scripts with the addition of
the --prefix flag (this was tricky for csv.py), which allows explicit
control over field field prefixes:
$ ./scripts/stack.py lfs.ci -o- --prefix=
function,frame,limit
lfs_alloc,288,1328
lfs_alloc_discard,8,8
lfs_alloc_findfree,16,32
...
$ ./scripts/stack.py lfs.ci -o- --prefix=wonky_
function,wonky_frame,wonky_limit
lfs_alloc,288,1328
lfs_alloc_discard,8,8
lfs_alloc_findfree,16,32
...
Guh
This may have been more work than I expected. The goal was to allowing
passing recursive results (callgraph info, structs, etc) between
scripts, which is simply not possible with csv files.
Unfortunately, this raised a number of questions: What happens if a
script receives recursive results? -d/--diff with recursive results?
How to prevent folding of ordered results (structs, hot, etc) in piped
scripts? etc.
And ended up with a significant rewrite of most of the result scripts'
internals.
Key changes:
- Most result scripts now support -O/--output-json in addition to
-o/--json, with -O/--output-json including any recursive results in
the "children" field.
- Most result scripts now support both csv and json as input to relevant
flags: -u/--use, -d/--diff, -p/--percent. This is accomplished by
looking for a '[' as the first character to decide if an input file is
json or csv.
Technically this breaks if your json has leading whitespace, but why
would you ever keep whitespace around in json? The human-editability
of json was already ruined the moment comments were disallowed.
- csv.py requires all fields to be explicitly defined, so added
-i/--enumerate, -Z/--children, and -N/--notes. At least we can provide
some reasonable defaults so you shouldn't usually need to type out the
whole field.
- Notably, the rendering scripts (plot.py, treemapd3.py, etc) and
test/bench scripts do _not_ support json. csv.py can always convert
to/from json when needed.
- The table renderer now supports diffing recursive results, which is
nice for seeing how the hot path changed in stack.py/perf.py/etc.
- Moved the -r/--hot logic up into main, so it also affects the
outputted results. Note it is impossible for -z/--depth to _not_
affect the outputted results.
- We now sort in one pass, which is in theory more efficient.
- Renamed -t/--hot -> -r/--hot and -R/--reverse-hot, matching -s/-S.
- Fixed an issue with -S/--reverse-sort where only the short form was
actually reversed (I misunderstood what argparse passes to Action
classes).
- csv.py now supports json input/output, which is funny.
Unifying these complicated attr-assigning flags across all the scripts
is the main benefit of the new internal Attr system.
The only tricky bit is we need to somehow keep track of all input fields
in case % modifiers reference fields, when we could previously discard
non-data fields.
Tricky but doable.
Updated flags:
- -L/--label -> -L/--add-label
- --colors -> -C/--add-color
- --formats -> -F/--add-format
- --chars -> -*/--add-char/--chars
- --line-chars -> -_/--add-line-char/--line-chars
I've also tweaked Attr to accept glob matches when figuring out group
assignments. This is useful for matching slightly different, but
similarly named results in our benchmark scripts.
There's probably a clever way to do this by injecting new by fields with
csv.py, but just adding globbing is simpler and makes attr assignment
even more flexible.
It doesn't really make sense to write to disk/trace files with multiple
threads, the result usually ends up clobbered and useless.
If we only pass disk/trace files to the first thread, the result is at
at least useable, even if it only represents 1/j tests.
This is actually quite a nice way to sample filesystem images in
multithreaded tests.
As a side effect, this also changes test.py/bench.py to no longer pass
-d/--disk or -t/--trace to runner queries, which is probably a good
thing? These should be ignored in queries anyways.
Found a bug in our toml parser that's difficult to work around:
defines.GC_FLAGS = """ => {
LFS_GC_MKCONSISTENT "GC_FLAGS": "blablabla",
| LFS_GC_LOOKAHEAD } // where did defines go?
"""
This appears to be this bug:
https://github.com/uiri/toml/issues/286
But since it was opened 4 years ago, I think it's safe to say this toml
library is now defunct...
---
Apparently tomllib/tomli is the new hotness, which started as tomli
before being adopt in Python 3.11 as tomllib. Fortunately tomli is still
maintained so we don't have to worry about Python versions too much.
Adopting tomli was relatively straightforward, the only hiccup being
that it doesn't support text files? Curious, but fortunately Python
exposes the underlying binary file handle in f.buffer.
Unfortunately the import sys in the argparse block was hiding missing
sys imports.
The mistake was assuming the import sys in Python would limit the scope
to that if block, but Python's late binding strikes again...
Moved local import hack behind if __name__ == "__main__"
These scripts aren't really intended to be used as python libraries.
Still, it's useful to import them for debugging and to get access to
their juicy internals.
It looks like the failure case in our scripts' subprocess stderr
handling was not tested well during a fix to stderr blocking (a735bcd).
This code was attempting to print stderr only if an error occured, but
with stderr=None this just results in a NoneType TypeError.
In retrospect, completely hiding stderr is kind of shitty if a
subprocess fails, but it doesn't seem possible to read from both stdin
and stderr with Python's APIs without getting stuck when the stderr's
buffer is full.
It might be possible to work around this with either multithreading,
select calls, or a temp file, but I'm not sure slightly less verbose
scripts are worth the added complexity in every single subprocess call.
For now just reverting to unconditionally forwarding stderr from the
child process. This is the simplest/most robust option.
This will probably only have niche uses, but may be useful for small
test sets or for running specific tests with -O-.
Though it is a bit funny that -q -O- turns test.py/bench.py into more or
less just a complicated way to run a C program.
A couple problems:
1. We should probably also support negative ranges, but this is a bit
annoying since we can't tell if the range is negative or positive
until expr evaluation.
2. Evaluating the range exprs at compile-time is inconsistent from other
C exprs in our tests/benches (normal defines, if filters, etc), and
severely limiting since we can't use other defines before the define
system is initialized.
2. Attempting to move these range exprs into their own lazily evaluated
functions does not seem tractable...
We'd need to evaluate defines to know how many permutations there
are, but how can we evaluate defines before knowing which permutation
we're on?
I think this circular dependency would make the permutation count
undecidable?
Even if we could move these exprs to their own lazily evaluated
functions (which would solve the inconsistency issue), the complexity
risks outweighing the benefit. Keep in mind it's useful if external
tools can parse our tests. So reverting for now.
Though I am keeping some of the refactoring in test.py/bench.py. Having
a special DRange type is useful if we ever want to add more define
functions in the future.
This enables full C exprs in test/bench define ranges by simply passing
them on to the C compiler.
So this:
defines.N = 'range(1,20+1)'
Becomes this, in N's define function:
if (i < 0 + ((((20+1)-1-(1))/(1) + 1))) return ((i-(0))*(1) + (1));
Which is a bit of a mess, but generates the correct range at runtime.
This allows for much more flexible exprs in range defines without
needing a full expr parser in Python.
Note though that we need to evaluate the range length at compile time.
This is notably before the test/bench define system is initialized, so
all three range args (start, stop, step) are limited to really only
simple C literals and exprs.
This seems like a more fitting name now that this script has evolved
into more of a general purpose high-level CSV tool.
Unfortunately this does conflict with the standard csv module in Python,
breaking every script that imports csv (which is most of them).
Fortunately, Python is flexible enough to let us remove the current
directory before imports with a bit of an ugly hack:
# prevent local imports
__import__('sys').path.pop(0)
These scripts are intended to be standalone anyways, so this is probably
a good pattern to adopt.
This matches the style used in C, which is good for consistency:
a_really_long_function_name(
double_indent_after_first_newline(
single_indent_nested_newlines))
We were already doing this for multiline control-flow statements, simply
because I'm not sure how else you could indent this without making
things really confusing:
if a_really_long_function_name(
double_indent_after_first_newline(
single_indent_nested_newlines)):
do_the_thing()
This was the only real difference style-wise between the Python code and
C code, so now both should be following roughly the same style (80 cols,
double-indent multiline exprs, prefix multiline binary ops, etc).
Mainly to avoid conflicts with match results m, this frees up the single
letter variables m for other purposes.
Choosing a two letter alias was surprisingly difficult, but mt is nice
in that it somewhat matches it (for itertools) and ft (for functools).
This moves all ckread-related logic behind the new opt-in compile-time
LFS_CKREADS flag. So in order to use ckreads you need to 1. define
LFS_CKREADS at compile time, and 2. pass LFS_M_CKREADS during
lfsr_mount.
This was always the plan since, even if ckreads worked perfectly, it
adds a significant amount of baggage (stack mostly) to track the
ck context of all reads.
---
This is the first non-trivial opt-in define in littlefs, so more test
framework features!
test.py and build.py now support the optional ifdef attribute, which
makes it easy to indicate a test suite/case should not be compiled when
a feature is missing.
Also interesting to note is the addition of LFS_IFDEF_CKREADS, which
solves several issues (and general ugliness) related to #ifdefs in
expression. For example:
// does not compile :( (can't embed ifdefs in macros)
LFS_ASSERT(flags == (
LFS_M_CKPROGS
#ifdef LFS_CKREADS
| LFS_M_CKREADS
#endif
))
// does compile :)
LFS_ASSERT(flags == (
LFS_M_CKPROGS
| LFS_IFDEF_CKREADS(LFS_M_CKREADS, 0)));
---
This brings us way back down to our pre-ckread levels of code/stack:
code stack
before-ckreads: 36352 2672
ckreads: 38060 (+4.7%) 3056 (+14.4%)
after-ckreads: 36428 (+0.2%) 2680 (+0.3%)
Unfortunately, we do end up with a bit more code cost than where we
started. Mainly due to code moving around to support the ckread
infrastructure:
code stack
lfsr_bd_readtag: +52 (+23.2%) +8 (+10.0%)
lfsr_rbyd_fetch: +36 (+5.0%) +8 (+6.2%, cold)
lfs_toleb128: -12 (-25.0%) -4 (-20.0%, cold)
total: +76 (+0.2%) +8 (+0.3%)
But oh well. Note that some of these changes are good even without
ckreads, such as only parsing the last ecksum tag.
code.py, specifically, was getting messed up by inconsequential GCC
objdump errors on Clang -g3 generated binaries.
Now stderr from child processes is just redirected to /dev/null when
-v/--verbose is not provided.
If we actually depended on redirecting stderr->stdout these scripts
would have been broken when -v/--verbose was provided anyways. Not
really sure what the original code was trying to do...
The original idea was to allow merging a whole bunch of different csv
results into a single lfs.csv file, but this never really happened. It's
much easier to operate on smaller context-specific csv files, where the
field prefix:
- Doesn't really add much information
- Requires more typing
- Is confusing in how it doesn't match the table field names.
We can always use summary.py -fcode_size=size to add prefixes when
necessary anyways.
Before, globs that match both the suite name and case name would cause
end up running the case twice. Which is a bit of a problem, since all
cases contain their suite name as a prefix...
test_f* => run test_files
|-> run test_files_hello
|-> run test_files_trunc
...
run test_files_hello
run test_files_trunc
...
Now we only run matching test cases if no suites were found.
This has the side-effect of making the universal glob, "*", equivalent
to no test ids, which is nice:
$ ./scripts/test.py -j -b '*' # equivalent
$ ./scripts/test.py -j -b #
This is useful for running a specific problematic test first before
running the all of the tests:
$ ./scripts/test.py -j -b test_files_trunc '*'
These really shouldn't be used all that often. Test filters are usually
used to protect against invalid test configurations, so if you bypass
test filters, expect things to fail!
But some filters just prevent test cases from taking too long. In these
cases being able to manually bypass the filter is useful for debugging/
benchmarking/etc...
Sometimes, if test_runner errors before running any tests, the last test
id can end up being None. This broke test output writing, which expected
to be able to parse an id. Instead we should just ignore the malformed
id (it's not like we can write anything relevant about any tests here),
and report it to the user at a higher level.
This acts as a marker to indicate a fuzz test. It should reference a
define, usually SEED, that can be randomized to get interesting test
permutations.
This is currently unused, but could lead to some interesting uses such
as time-based fuzz testing. It's also just useful for inspecting the
tests (make test-list).
Now that we have ~20 minutes of tests, it's good to know _why_ the tests
take ~20 minutes, and if this time is being spent well.
This adds the field test_time to test.py's -o/--output, which reports
the runtime of each test in seconds. This can be organized by suite,
case, etc, with our existing csv scripts.
Note I've limited the precision to only milliseconds (%.6f).
Realistically, this is plenty of precision, and with the number of tests
we have extra digits can really add up!
lines bytes
test.csv before: 525593 58432541 56MiB
test.csv full precision: 525593 (+0.0%) 69817693 67MiB (+19.5%)
test.csv milli precision: 525593 (+0.0%) 63162935 60MiB (+8.1%)
It still takes a bit of time to process this (50.3s), but now we can see
the biggest culprits of our ~20 minute test time:
$ ./scripts/summary.py test.csv -bcase -ftest_time -S
case test_time
...
test_fwrite_hole_compaction 74.4
test_fwrite_incr 109.7
test_dirs_mkdir_fuzz 115.3
test_fwrite_overwrite_compaction 132.4
test_rbyd_fuzz_append_removes 134.0
test_rbyd_fuzz_mixed 136.3
test_rbyd_fuzz_sparse 137.4
test_fwrite_w_seek 144.1
test_rbyd_fuzz_create_deletes 144.8
test_dirs_rm_many_backwards 208.4
test_dirs_rm_many 273.8
test_fwrite_fuzz_unaligned 283.2
test_dread_recursive_rm 316.7
test_fwrite_fuzz_aligned 551.0
test_dirs_general_fuzz 552.8
test_dirs_rm_fuzz 632.7
test_fwrite_reversed 719.0
test_dirs_mv_fuzz 1984.8
TOTAL 7471.3
Note this machine has 6 cores, 12 hthreads, 7471.3/60/6 => 20.8m, which
is why I don't run these tests single threaded.
There was no check on context > stdout, so requesting more context than
was actually printed by the test could result in a negative value.
Python "helpfully" interpreted this as a negative index, resulting in
somewhat random context lengths.
This, combined with my tendency to just default to a large number like
--context=100, led to me thinking a test was printing much less than it
actually was...
Don't get me wrong, I love Python, and I think Python's negative indices
are a clever way to add flexibility to slice notation, but the
value-dependent semantics are a pretty unfortunate footgun...
While the -f/--fail logic was correctly terminating the test.py/bench.py
runner thread, it was not terminating the actual underlying test
process. This was causing test.py/bench.py to hang until the test runner
completed all pending tests, which could take quite some time.
This wasn't noticed earlier because test.py/bench.py still reports the
test as failed, and most uses of -f/--fail involve specifying a specific
test case, which usually terminates quite quickly.
What's more interesting is this termination logic was copied from the
handling of ctrl-C/SIGINT/KeyboardInterrupt, but this issue is not
present there because SIGINT would be sent to all processes in the
process tree, terminating the child process anyways.
Fixed by adding an explicit proc.kill() to test.py/bench.py before
tearing down the runner thread.
This is a condition for specifically the -O- pattern. Doing anything
fancier would be too much, so anything clever such as -O/dev/stdout
will still be clobbered.
This was a common enough pattern and the status updates clobbering
stdout was annoying enough that I figured this warranted a special case.
Christopher Haster