If a test caused an abort condition to trigger, a pseudoresult
"igt@runner@aborted" would be generated, detailing which test caused
the condition. That was implemented in a simpler world where there
were no dynamic subtests and CI systems (i915-CI in particular) didn't
have much test resuming happening.

The generated pseudoresult was completely unable to point to a
particular dynamic subtest if the executed subtest had any, and there
could inherently be only one of them. When combined with the fact that
the pseudoresult doesn't have logs that could be used to automatically
point to particular bug reports, that made results filtering by
CIBugLog difficult.

With socket communications it's easier to inject information "in
between" test's logs and result overriding is designed into the
system. Thus, when using socket communications, this patch series will
make the abort-condition-causing test's result 'abort' instead of
using that pseudoresult.

The flow becomes a little bit messier with monitor_output() gaining
more and more out-parameters but those can be cleaned later.

v2: Handle abort conditions between dynamic subtests

Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arek@hiler.eu>
Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Acked-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

Abort-condition checking at runtime will later use the helper
differently.

Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arek@hiler.eu>
Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Acked-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

When processing a resultoverride packet, the override is supposed to
apply only to the most recent subtest or dynamic subtest. Properly
clear the override so only those results get overridden.

Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arek@hiler.eu>
Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Acked-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

Don't leave the execution in a "please resume me" state if bootup
causes an abort condition. Especially handle the case of abort on
bootup when resuming correctly, so that it doesn't attempt to run a
test on a tainted kernel if we've explicitly configured the runner to
not execute when there's a taint.

v2: Fudge the results directory instead to get the desired results:
    runner exits with nonzero, and resuming exits with "all done" instead
    of executing anything.

v3: Use faccessat instead of open+close, use less magic strings,
    remember to close fds (Chris)

v4: Use GRACEFUL_EXITCODE in monitor_output, remove the 'resuming'
    field (why was it a double?!). (Ryszard)
    Stop trying to execute if all tests are already run, to avoid a
    crash in environment validation.

v5: Remember to git add so the 'resuming' field really gets
    removed. (Kamil)
    Use 0.000 in the printf format directly instead of formatting 0.0
    to %.3f. (Kamil)

Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arek@hiler.eu>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Cc: Ryszard Knop <ryszard.knop@intel.com>
Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

Disk space usage tracking is supposed to be per (dynamic)
subtest. Remember to reset the usage tracker when subtest changes.

Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arek@hiler.eu>
Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>

When socket communications are used, the journal is left empty. When
processing results, reading an empty journal will add a test result
for the requested test, which can be incorrect when using testlists
with wildcards.

Testlists with wildcards were not the intended use of testlists but
they are used and thus this is considered a fix for a regression.

Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Nicholas Choi <Nicholas.Choi@amd.com>
Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Acked-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

The matches object's internals weren't released.

v2: Don't leak in the other branch either. (Kamil)

Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arek@hiler.eu>
Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

With a couple of fixes, using socket comms is now suitably ready for
prime time.

This reverts commit 372c5622.

Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arek@hiler.eu>
Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

If a packet of invalid size is received, inject a message in the dump,
override result to warn, and continue grabbing packets.

Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arek@hiler.eu>
Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

Stream-based receiving was able to use buffers of any size for read(),
but with datagrams we actually need to prepare for actual sizes. In
practice, some file dumps from tests come as single log packets of a
couple of kB.

v2: Use a KB() macro for the buffer size (Kamil).

Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arek@hiler.eu>
Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

Currently, having i915 CI systems test the socket communications when
testing is impossible. If a test hangs completely, the incomplete set
of results are processed into results.json with an older IGT version
from a previous post-merge build, which is unable to parse socket
communications. Disable them for now.

This commit will get reverted after suitable testing has been possible
to execute.

v2: Rename the enable var, explain its presence further in a comment
    (Kamil)

Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Acked-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

When debugging, being able to read what is sent via socket comms is
very useful.

Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arek@hiler.eu>
Acked-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

Test that initializing resumes from various states of execution gets
the correct resume-state up.

v2: Flesh out commit message (Kamil)

Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arek@hiler.eu>
Acked-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

Set up a socket and pass it to the tests. Dump received communications
to disk.

When generating results.json, use socket comms if it exists.

Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arek@hiler.eu>
Acked-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

Jake Freeland authored 2 years ago and Petri Latvala committed 2 years ago

Use #ifdef __linux__ to limit all Linux-specific headers to
Linux systems

Signed-off-by: Jake Freeland <jfree@freebsd.org>
Acked-by: Petri Latvala <petri.latvala@intel.com>

Per Mauro's review, add some --environment tests:
- Check if -e/--environment works,
- Check both KEY=VALUE and KEY,
- Check if resumes load the variables correctly.

v2: Removed unnecessary checks, misc cleanup (Mauro)

Settings serialization now creates a separate file, environment.txt,
which contains KEY=VALUE pairs of environment variables to use for test
process spawning. Only vars created with --environment are serialized.
Also add misc warning fixes.

v2:
- Simplify env var reading from files (Mauro)
- Don't align settings loading block (Mauro)
- Make filenames static const (Mauro)

Allows specifying additional environment variables to be set for the
launched test processes. This commit introduces the argument parsing.

v2:
- Moved warning fixes from later commits here
- igt_list_empty_or_null -> igt_list_empty (Mauro)
- Optimized the parser, more error handling (Mauro)

For easier error logging, make the usage() function variadic and pass
its arguments to vfprintf. Additionally, reorder its arguments so that
it visually matches all the printf functions.

v2:
- usage(msg, ...) -> usage(stderr) (Maira)
- __attribute__ ((format (...))) (Petri)

That function does not actually free the settings pointer - rename it
for clarity.

Dragoon Aethis authored 2 years ago and Petri Latvala committed 2 years ago

These values were immediately overwritten or left unused, remove them.

Signed-off-by: Ryszard Knop <ryszard.knop@intel.com>
Reviewed-by: Petri Latvala <petri.latvala@intel.com>

Dragoon Aethis authored 2 years ago and Petri Latvala committed 2 years ago

The intent here most likely was that if strchr returns NULL, the error
below is printed out, but the condition was to check if the provided
buffer was not NULL.

Signed-off-by: Ryszard Knop <ryszard.knop@intel.com>
Reviewed-by: Petri Latvala <petri.latvala@intel.com>

Mauro Carvalho Chehab authored 2 years ago and Petri Latvala committed 2 years ago

In order to prepare to execute the code coverage scripts from the
PATH, change the logic at settings in order for it to seek for
the script in the PATH, if it doesn't contain any directories
on its filename.

Note: file search routines were moved (unchanged) on this path,
in order to avoid forward prototype declarations.

Reviewed-by: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

igt_runner converts passing test results to warn if a test outputs
stuff to stderr. It also converts results if kernel logs contain
warnings to dmesg-warn or dmesg-fail, depending on the test's own
result.

Perform this same conversion to subtests even if they contain dynamic
subtests, now that we have the capability of keeping those around.

Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Swati Sharma <swati2.sharma@intel.com>
Reviewed-by: Swati Sharma <swati2.sharma@intel.com>

Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arek@hiler.eu>
Reviewed-by: Swati Sharma <swati2.sharma@intel.com>

In preparation for testing the rest of the options for prune_mode,
rename the current dynamic subtest result test to reflect the option
value.

Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arek@hiler.eu>
Reviewed-by: Swati Sharma <swati2.sharma@intel.com>

Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arek@hiler.eu>
Reviewed-by: Swati Sharma <swati2.sharma@intel.com>

Allow finer control of reporting dynamic subtests instead of
unconditionally assuming that the main subtest result and logs are
uninteresting if the subtest has dynamic subtests.

The default is still to remove subtest results when the subtest has
dynamic subtests. Other options are:

keep-subtests: Remove the dynamic subtests instead, for cases when a
stable test count is more important.

keep-all: Remove nothing.

keep-requested: Remove the results that were not directly requested to
be executed. This option is useful in cases where the test selection
is a hand-picked mix of subtests and particular dynamic subtests.

Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arek@hiler.eu>
Reviewed-by: Swati Sharma <swati2.sharma@intel.com>

Instead of stopping execution on resume-init success, stop on
resume-init failure like intended.

Fixes: 4b88a925 ("runner: check if it has root permissions")
Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Reviewed-by: Mauro Carvalho Chehab <mchehab@kernel.org>

Mauro Carvalho Chehab authored 3 years ago and Petri Latvala committed 3 years ago

Ensure that "-o" parameter will also cleanup the contents of the
code coverage results directory.

Reviewed-by: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

Mauro Carvalho Chehab authored 3 years ago and Petri Latvala committed 3 years ago

The gcc compiler has a feature that enables checking the code coverage
in runtime[1].

[1] See https://www.kernel.org/doc/html/latest/dev-tools/gcov.html



The Linux Kernel comes with an option to enable such feature:

	./scripts/config -e DEBUG_FS -e GCOV_KERNEL

The driver's Makefile also needs change to enable it. For instance, in
order to enable GCOV for all DRM drivers, one would need to run:

	for i in $(find drivers/gpu/drm/ -name Makefile); do
                sed '1 a GCOV_PROFILE := y' -i $i
        done

This patch adds support for it by:

a) Implementing a logic to cleanup the code coverage counters via sysfs;
b) Calling a script responsible for collecging code coverage data.

The implementation works with two modes:

1) It zeroes the counters, run all IGT tests and collects the code
   coverage results at the end.

   This implies that no tests would crash the driver, as otherwise the
   results won't be collected;

   This is faster, as collecting code coverage data can take several
   seconds.

2) For each test, it will clean the code coverage counters, run the and
   collect the results.

   This is more reliable, as a Kernel crash/OOPS won't affect the
   results of the previously ran tests.

Reviewed-by: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

Mauro Carvalho Chehab authored 3 years ago and Petri Latvala committed 3 years ago

Without root permissions, most IGT tests won't actually run, but they
would be displayed at the runner's output as if everything went fine.

In order to avoid that, check if one attempts to run IGT without root
permission. Such check can be disbled with a new command line option:

	--allow-non-root

As runner_tests runs as non-root, most unit tests need to pass
--allow-non-root in order for them to not return an error.

Reviewed-by: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

Chris Wilson authored 3 years ago and Ashutosh Dixit committed 3 years ago

Add typechecking to the min/max macros and make their locals truly
unique-ish to reduce the risk of shadowing.

v2: small bug fix, write also height coordinate on rotation
    test. (jheikkil)
v3: Fix up a couple of other max/max_t instances (Ashutosh)

Signed-off-by: Juha-Pekka Heikkilä <juha-pekka.heikkila@intel.com>
Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
Signed-off-by: Chris Wilson <chris.p.wilson@intel.com>
Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>

Now that autotools is gone and you always build to a dedicated build
directory with meson, we can remove .gitignore files that only had the
purpose of ignoring built binaries in the source directories.

Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arek@hiler.eu>
Reviewed-by: Arkadiusz Hiler <arek@hiler.eu>

The SIGHUP handling was incorrectly done only when log level was at
least 'normal'.

Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arek@hiler.eu>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Commit 10c83dfa ("runner: Introduce a way to stop testing without
marking tests incomplete") added graceful exiting by signaling the
runner with SIGHUP, marking the currently running test as 'notrun'
instead of 'incomplete'. The result handling didn't handle dynamic
subtests though, so the currently executing dynamic subtest was still
marked as 'incomplete'. Handle that now similarly as handling the
'abort' result.

Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arek@hiler.eu>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arek@hiler.eu>
Reviewed-by: Arkadiusz Hiler <arek@hiler.eu>

Killing igt_runner with SIGHUP will now still kill the currently
running test, but it will mark that test as being "notrun" instead of
"incomplete". This allows for external tools to interrupt the testing
without messing the results.

Incidentally, Intel CI's testing procedures occasionally falsely
determine that the machine being tested is unreachable and as its next
step, will ssh in and issue a reboot in preparation for the next round
of testing, causing igt_runner to be killed with a SIGHUP...

v2:
 - Fix typo SIGUP -> SIGHUP
 - Make runner print that a graceful exit will be done
 - Explain the code flow regarding handling of signals to the runner process
 - Use GRACEFUL_EXITCODE instead of -SIGHUP directly

Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Tomi Sarvela <tomi.p.sarvela@intel.com>
Cc: Arkadiusz Hiler <arek@hiler.eu>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>