| Age | Commit message (Collapse) | Author |
|---|
|
See also: commit 0e6457f1bfe2 ("lib: Don't dump log buffer when
dynamic subtest failure is inherited")
This is quite an explicit top-to-bottom test that we don't get an
incorrect warn result for an innocent dynamic subtest. It is tested
here in runner_test instead of testing in lib/tests that the extra
lines are not printed, because not printing the extra lines is an
implementation detail that might change later. The issue is after all
about test results parsed by igt_runner.
v2: Squash adding the new mockup test binary to this commit
Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
|
|
We have some handcrafted test binaries in runner/testdata/ for runner
testing, and hardcoded numbers for the total amount of subtests and
test binaries all over the runner's unit tests. Replace magic numbers
with clear defines so new tests can easily be added.
Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
|
|
A simple test output with numbers from 1 to 255, both in plain text
form and as a single byte with that particular value.
Note that the json spec doesn't require \u-encoding for characters
other than '"', '\' and the range U+0000 to U+001F, the raw
non-\u-encoded UTF-8 in the reference.json file for bytes 128 and up
is what libjson-c outputs for those codepoints and is valid.
The validity of the json file can be verified with iconv, i.e.
$ iconv -f UTF-8 reference.json -o /dev/null && echo it is utf-8
v2: Rebase over dynamic subtest tests, trivial
Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
|
|
Sometimes tests output garbage (e.g. due to extreme occurrences of
https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/55) but we
need to present the garbage as results.
We already ignore any test output after the first \0, and for the rest
of the bytes that are not directly UTF-8 as-is, we can quite easily
represent them with two-byte UTF-8 encoding.
libjson-c already expects the string you feed it through
json_object_new_string* functions to be UTF-8.
v2: Rebase, adjust for dynamic subtest parsing
Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
|
|
Multiple different subtests can have a dynamic subtest with the same
name. Add a test to make sure we correctly delimit the output parsing.
Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
|
|
Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
|
|
Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
|
|
Dynamic subtests now include more output, change the dynamic subtest
parsing test accordingly.
Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
|
|
On an assertion failure, the string "Subtest xyz failed" is
printed. Make sure we don't match that for SUBTEST_RESULT, or the
equivalent for dynamic subtests.
Parsing the results already explicitly searched for the proper result
line, the difference is when we delimit output up to "the next line of
interest".
Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
|
|
Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
|
|
The first dynamic part now starts its reported output at the beginning
of its subtest's output, and the last dynamic part ends its reported
output at the end of its subtest's output.
v2: Rebase
Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
|
|
v2: Rebase
Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
|
|
v2: Rebase
Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
|
|
v2: Use an enum to select a pattern string for asprintf (Arek)
Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
|
|
Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
|
|
The default timeout is 30s.
Recently we see more `ninja test` invocation failing due to runner tests
timing out. This happens for slower environments, i.e. running the
cross-compiled binaries through qemu.
Freedesktop CI/CD runners are shared machines with a lot of cores and
they accept many parallel jobs coming from multiple projects. There
are no resources guarantees, which leads to the sporadic slowness.
Runner tests are taking 10x longer than the next slowest test invoked by
`ninja test` on a typical run. They also seem more prone to resource
thrashing by other processes on the machine, probably due to heavier
reliance on disk IO.
Let's just give them proportional leeway when it comes to timing out.
Cc: Petri Latvala <petri.latvala@intel.com>
Issue: https://gitlab.freedesktop.org/freedesktop/freedesktop/issues/197
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Petri Latvala <petri.latvala@intel.com>
|
|
While the originally written timeout for process killing (2 seconds)
was way too short, waiting indefinitely is suboptimal as well. We're
seeing cases where the test is stuck for possibly hours in
uninterruptible sleep (IO). Wait a fairly longer selected time period
of 2 minutes, because even making progress for that long means the
machine is in bad enough state to require a good kicking and booting.
v2:
- Abort quicker if kernel is tainted (Chris)
- Correctly convert process-exists check with kill() to process-does-not-exist
Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
|
|
The split to timeout intervals was made to accomodate for watchdogs
that cannot use a timeout as high as we wanted. Actually using that
feature requires us to ping the watchdog every interval even though we
handle actual timeouting after all intervals are used up.
Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
|
|
Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
|
|
If binary 'bin' has a subtest 'sub', which has dynamic subtests 'foo'
and 'bar', results.json will now have "subtests" by the names
igt@bin@sub@foo and igt@bin@sub@bar, with data as expected of normal
subtests.
Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
|
|
Don't add timestamps when printing that we cannot execute a binary
from a child (post fork-failed-execv). Timestamps were meant for
runner's direct output only, and this was accidentally converted.
v2: Rephrase commit message (Arek)
Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
|
|
Since we actually include the output before the subtest begins now,
add it to the reference.jsons where applicable.
Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
|
|
Instead of searching back and forth for proper lines, first find all
lines that we could be interested in with one pass through the output,
and use the positions of found lines to delimit the extracted outputs.
Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
|
|
If a test was attempted but didn't actually exist, make it result in a
skip instead of a notrun. This is to differentiate them from the tests
that we didn't even attempt, like tests after getting a machine
hang. This will improve handling of subtests for GEM engines that
don't exist.
Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Martin Peres <martin.peres@linux.intel.com>
Acked-by: Martin Peres <martin.peres@linux.intel.com>
|
|
When our watchdog expires and we declare the test has timed out, we send
it a signal to terminate. The test will produce a backtrace upon receipt
of that signal, but often times (especially as we do test and debug the
kernel), the test is hung inside the kernel. So we need the kernel state
to see where the live/deadlock is occuring. Enter sysrq-t to show the
backtraces of all processes (as the one we are searching for may be
sleeping).
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Petri Latvala <petri.latvala@intel.com>
|
|
If an output (out.txt or err.txt) is completely empty, we handle the
parsing just fine as is, but we end up assuming that if journal says
we have a subtest, that subtest printed that it started. We have one
case where out.txt was empty and all other files were intact (ran out
of disk?)
All other paths that expect certain texts handle failures finding them
properly apart from subtest result processing, which happily passed
along a NULL pointer as a string to json. After handling that case,
the processing of said weird case proceeded fine and produced correct
results.
Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
|
|
A minor refinement to remove the trailing spaces after converting the
NUL-terminators to spaces.
v2: Beware the crafty filename entirely composed of spaces.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Petri Latvala <petri.latvala@intel.com>
Reviewed-by: Petri Latvala <petri.latvala@intel.com>
|
|
/proc/$pid/cmdline is the entire argv[] including NUL-terminators.
Replace the NULs with spaces so we get a better idea of who the
signaler was, as often it is a subprocess (such as a child of sudo,
or worse java).
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Petri Latvala <petri.latvala@intel.com>
Reviewed-by: Petri Latvala <petri.latvala@intel.com>
|
|
We want to know who sent us the fatal signal, for there are plenty of
fingers to go around.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Petri Latvala <petri.latvala@intel.com>
|
|
If the network goes down while testing, CI tends to interpret that as
the device being down, cutting its power after a while. This causes an
incomplete to an innocent test, increasing noise in the results.
A new flag to --abort-on-monitored-error, "ping", uses liboping to
ping a host configured in .igtrc with one ping after each test
execution and aborts the run if there is no reply in a hardcoded
amount of time.
v2:
- Use a higher timeout
- Allow hostname configuration from environment
v3:
- Use runner_c_args for holding c args for runner
- Handle runner's meson options in runner/meson.build
- Instead of one ping with 20 second timeout, ping with 1 second timeout
for a duration of 20 seconds
v4:
- Rebase
- Use now-exported igt_load_igtrc instead of copypaste code
- Use define for timeout, clearer var name for single attempt timeout
Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Martin Peres <martin.peres@linux.intel.com>
Cc: Tomi Sarvela <tomi.p.sarvela@intel.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
|
|
Very handy for correlating events between different logs.
This generate igt_runner0.txt output like:
[28.112360] Initializing watchdogs
[28.112424] /dev/watchdog0
[28.114069] [001/269] (960s left) core_auth (basic-auth)
Starting subtest: basic-auth
Subtest basic-auth: SUCCESS (0.000s)
[28.224898] [002/269] (960s left) debugfs_test (read_all_entries)
Starting subtest: read_all_entries
Subtest read_all_entries: SUCCESS (0.035s)
The subtest logs are separate (not part of the runner's logging per-se),
but the flow of events is clear enough from the runner's timestamp for
now.
v2: Concatenate split messages into a single call (so that the timestamp
is only added once!)
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Petri Latvala <petri.latvala@intel.com>
Reviewed-by: Petri Latvala <petri.latvala@intel.com>
|
|
There are few short windows of opportunity when watchdogs are primed but
there is no signal handling in place, so the process may exit without
proper shutdown sequence.
This patch rearranges the existing code so that we set up the signalfd
and BLOCK the signals before setting up watchdogs and UNBLOCK only after
the watchdogs are closed properly.
If igt_runner exits due to signal, non-zero status code is returned.
v2: more error handling and minor touch ups (Simon)
Cc: Petri Latvala <petri.latvala@intel.com>
Cc: Simon Ser <simon.ser@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Simon Ser <simon.ser@intel.com>
Reviewed-by: Petri Latvala <petri.latvala@intel.com>
|
|
instead of being closed normally on a graceful code path
Cc: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Simon Ser <simon.ser@intel.com>
Reviewed-by: Petri Latvala <petri.latvala@intel.com>
|
|
Setting the watchdog fd lists to NULL for extra fireworks if accessed
unintentionally.
Cc: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Simon Ser <simon.ser@intel.com>
Reviewed-by: Petri Latvala <petri.latvala@intel.com>
|
|
Meson emits a warning because we use options beginning with "build_":
DEPRECATION: Option uses prefix "build_", which is reserved for Meson. This will become an error in the future.
Rename our options so that we don't use the Meson-reserved prefix.
Signed-off-by: Simon Ser <simon.ser@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
|
|
If provided an empty blacklist let's fail instead of returning a value
of uninitialized variable.
Cc: Oleg Vasilev <oleg.vasilev@intel.com>
Cc: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Petri Latvala <petri.latvala@intel.com>
|
|
If we don't get values from the JSON strlen() and strcmp() on them would
dereference NULL pointer.
Cc: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Petri Latvala <petri.latvala@intel.com>
|
|
intervals_left got initialized only when when we had a timeout exceeding
watchdog capabilities, meaning we had to use multiple shorter intervals
by moving intervals_left = timeout_intervals down we are always
initializing it
Cc: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Petri Latvala <petri.latvala@intel.com>
|
|
Feed the curious ones, aid the troubleshooters.
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Simon Ser <simon.ser@intel.com>
Reviewed-by: Petri Latvala <petri.latvala@intel.com>
|
|
Default handler for SIGHUP is also terminating the process, so let's
mask it and handle it manually, like the rest of the bunch.
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Simon Ser <simon.ser@intel.com>
Reviewed-by: Petri Latvala <petri.latvala@intel.com>
|
|
If write or ioctl on a watchdog ever fails it will be logged.
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Simon Ser <simon.ser@intel.com>
Reviewed-by: Petri Latvala <petri.latvala@intel.com>
|
|
Currently, runner already collects all subtest names into job_list.
--list-all allows to output it to stdout.
--blacklist option takes a filename as an argument and adds all regexes
from that file to the exclusion list.
v2:
- Update exclude/include regex matches for tests without
subtests to be matched with pigtit-like name (Petri)
- Replace relative paths with those formatted with testdatadir (Petri)
- Minor codestyle changes
v3:
- Print test names in lowercase (Petri)
v4:
- Replaced custom str_to_lower with generate_piglit_name (Petri)
- Minor codestyle changes
Cc: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Oleg Vasilev <oleg.vasilev@intel.com>
Reviewed-by: Petri Latvala <petri.latvala@intel.com>
|
|
Some libraries (looking at you, GnuTLS) may leave fds opened after the
implicitly called library constructor. We don't have full control over
them as they may be dependencies of our dependencies and may get pulled
in if the user's and distribution's compile/configure/USE are just
right.
Let's close all the non-standard fds ahead of executing anything in the
igt_runner tests, so we can test for descriptor leakage caused by any of
the igt_runner code-paths exercised there.
Cc: Antonio Argenziano <antonio.argenziano@intel.com>
Cc: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Antonio Argenziano <antonio.argenziano@intel.com>
Reviewed-by: Simon Ser <simon.ser@intel.com>
|
|
As seen in fallout with the i915 engine discovery patchset, subtests
can in very dire cases be entered multiple times, when the for loop
they're in doesn't progress. That is of course a bug in IGT that needs
to be fixed, but if (when) that happens again, don't make results take
a million years to process.
This patch has been tested to produce good results from the
corresponding bad runs.
Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Tomi Sarvela <tomi.p.sarvela@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
|
|
Meson 0.47 comes with a new type of option called 'feature' so instead of:
type : 'combo',
value : 'auto',
choices : ['auto', 'true', 'false'],
We can:
type : 'feature',
The main difference is that the feature takes auto, enabled and disabled
instead of auto, true and false.
get_option() on a feature returns opaque object that can be passed as
a 'required' argument of a dependency. Auto is equivalent to 'required
: false', enabled is equivalent to 'required : true' and disabled
introduces new behavior forcing the dependency to be considered not
found.
This allows us to streamline a lot of logic regarding optional IGT
features.
This patch bumps required meson version to 0.47.0
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Petri Latvala <petri.latvala@intel.com>
Cc: Simon Ser <simon.ser@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
|
|
POSIX ERE, while pretty standard is also very old and severely limited.
In fact, it's so limited that Intel CI's own test blacklist,
tests/intel-ci/blacklist.txt, doesn't even work with it due to the lack
of support for backreferences.
Since we're already using glib in other parts of igt, let's just start
using glib's regular expression matching instead of the POSIX regex API.
This gives us full perl compatible regular expressions and in turn, also
gives us python compatible regular expressions to match piglit.
Reviewed-by: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Lyude Paul <lyude@redhat.com>
|
|
Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110248
Cc: Eero Tamminen <eero.t.tamminen@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
|
|
Quoting lib/igt_core.c:
* - Code blocks with magic control flow are implemented with setjmp() and
* longjmp(). This applies to #igt_fixture and #igt_subtest blocks and all the
* three variants to finish test: igt_success(), igt_skip() and igt_fail().
* Mostly this is of no concern, except when such a control block changes
* stack variables defined in the same function as the control block resides.
* Any store/load behaviour after a longjmp() is ill-defined for these
* variables. Avoid such code.
*
* Quoting the man page for longjmp():
*
* "The values of automatic variables are unspecified after a call to
* longjmp() if they meet all the following criteria:"
* - "they are local to the function that made the corresponding setjmp() call;
* - "their values are changed between the calls to setjmp() and longjmp(); and
* - "they are not declared as volatile."
We have quite a few igt_subtest_group that define local variables to
share the state between subtest and fixture (e.g. set open() in subtest,
close() in the following fixture), causing the contents to be undefined.
We can allocate some of them using malloc and make the others volatile
to be back on the defined grounds.
Cc: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Petri Latvala <petri.latvala@intel.com>
|
|
Using a filter that doesn't match any test name resulted in the runner
silently failing. Print an error message so that the user understands
why the runner fails.
Signed-off-by: Simon Ser <simon.ser@intel.com>
Reviewed-by: Petri Latvala <petri.latvala@intel.com>
|
|
Tests that eat all of the RAM and then some to invoke the oom-killer
deliberately sometimes cause extra casualties. Make sure the runner
stays alive.
Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
|
