When --dry-run option is used, there's no need to require root,
as no tests will actually be executed.

Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

Petri Latvala authored 1 year ago and Mauro Carvalho Chehab committed 1 year ago

Now that runner normalizes no-subtest entries in testlists, make sure
the name matches what's available.

Acked-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Signed-off-by: Petri Latvala <adrinael@adrinael.net>

Petri Latvala authored 1 year ago and Mauro Carvalho Chehab committed 1 year ago

Add unit tests to check if testlist will be expanded if a
subtest is blacklisted.

Acked-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Signed-off-by: Petri Latvala <adrinael@adrinael.net>

Petri Latvala authored 1 year ago and Mauro Carvalho Chehab committed 1 year ago

As the syntax for "all subtests" and "test that doesn't have subtests"
is the same, check the subtest listing when building the execution
plan. Doing that makes it possible to still have "all subtests" in
testlists (albeit not originally designed to support that) and enables
blocklisting particular subtests.

Signed-off-by: Petri Latvala <adrinael@adrinael.net>
Reviewed-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Arkadiusz Hiler <arek@hiler.eu>
Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
Closes: #149

Chris Wilson authored 1 year ago and Kamil Konieczny committed 1 year ago

As well as the task and cpu state we show, include the GPU state now
available via sysrq upon a runner abort (typically a timeout for an
unresponsive driver).

Cc: Petri Latvala <adrinael@adrinael.net>
Signed-off-by: Chris Wilson <chris.p.wilson@intel.com>
Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>

This reverts commit 3bd8bf9b.

After some tests it seems that runner is not reporting failures
properly from i915_selftest due to this change, also after
re-thinking I come to conclusion that clipping may be applied
after runner/resume processes a tests dmesg from a run into
rusults.json

Fixes: 3bd8bf9b ("runner: check disk limit at dumping kmsg")
Cc: Andrzej Hajda <andrzej.hajda@intel.com>
Cc: Petri Latvala <adrinael@adrinael.net>
Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>

It was reported that kernel dumps can grow beyond disk limit size
so add checks for it and report error if that happen.

v2: return number of written bytes (Petri)

Reported-by: Karol Krol <karol.krol@intel.com>
Ref: #129


Cc: Petri Latvala <adrinael@adrinael.net>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Reviewed-by: Petri Latvala <adrinael@adrinael.net>
Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>

Minimal pipeline s now failing:
	https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/38824636



This doesn't seem to be related to the patches at the series from
where the issue was reported, but addressing it is simple enough:
just remove the error and change the logic to properly validate
the possible options.

Acked-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

Changes on tests may break --list-subtests, if they, for instance,
someone adds a code outside igt_subtests that depends on igt_fixture
to run, causing:

        Received signal SIGABRT.
        Stack trace:
         #0 [fatal_sig_handler+0x17b]
         #1 [__sigaction+0x50]
         #2 [__pthread_kill_implementation+0x10c]
         #3 [gsignal+0x16]
         #4 [abort+0xd7]
         #5 [__assert_fail_base.cold+0xf]
         #6 [__assert_fail+0x46]
         #7 [internal_assert+0xe5]
         #8 [igt_fail+0xea]
         #9 [__igt_fail_assert+0x10e]
         #10 [xe_number_hw_engines+0x6b]
         #11 [__igt_unique____real_main1528+0x1ac0]
         #12 [main+0x23]
         #13 [__libc_start_call_main+0x80]
         #14 [__libc_start_main+0x89]
         #15 [_start+0x25]
        Aborted (core dumped)

IGT runner currently prints the message, but don't ignore the
error. That prevents scripts to rely on the IGT testlist
output, as the list will miss subtests.

So, instead, return an error.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Reviewed-by: Mauro Carvalho Chehab <mchehab@kernel.org>

When results generation fails exit with code 3, it will help
to differentiate from failed exection at resume runs. Added
also some debug messages at different error points.

Cc: Petri Latvala <adrinael@adrinael.net>
Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Reviewed-by: Petri Latvala <adrinael@adrinael.net>
Reviewed-by: Mauro Carvalho Chehab <mchehab@kernel.org>

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>