Sometimes an error happens in kernel or in test that leaves
output files in corrupted or incorrect state. While runner or
resume will just move on to executing next test, when generating
results it could end up with no results.json

  Try processing outputs a little more persistently and use any
output file left there, even if only dmesg.txt. Also, when no
useful output files were present, instead of breaking out add
notrun.

  Inform about processing results for each test so a problem
could be spotted more easily.

v2: removed ')' from 'notrun\n)' (Kamil)
 using bool var, added more prints about errors (Ryszard)
v3: reused open_for_reading, removed bool var (Krzysztof)
 closing only positive fds[] in close_outputs(), checking
 file sizes also if all opens succeeded (Kamil)
v4: reverting to v2 and addresing review comments (Krzysztof)
 closing only already opened file, drop early return when empty
 output files as this changes run status (Kamil)
v5: reverting to returning fals...

Peter Senna Tschudin authored 2 weeks ago and Kamil Konieczny committed 2 weeks ago

This patch modifies igt_runner to support runner_kmemleak() calls. By
default, kmemleak scanning is disabled, so new command-line options are
introduced to enable it:
 * -k, -k<option>, --kmemleak, --kmemleak=<option>

The available options are:
 * once: Do single kmemleak scan after last test in the test list
 * each: Perform a kmemleak scan after each test completes

By default, kmemleak scanning is disabled. If any kmemleaks are
detected, they will be saved in the igt_runner results directory under
kmemleak.txt.

Additionally, this patch updates serialize_settings() and
read_settings_from_file() to persist igt_runner settings across runs.
This allows settings to be saved when running igt_runner --dry-run and
later restored when executing igt_resume.

The unit tests for igt_runner have been extended to verify:
 * Kmemleak scans are disabled by default
 * Kmemleak scans can be enabled via command-line arguments
 * The kmemleak setting is correctly saved to and restored from disk

To test the new -k command-line option, this patch appends "--overwrite"
to *argv[] in runner_test.c to expand the argument array. This approach
avoids a major refactor of how *argv[] is defined across the file while
keeping the changes isolated to unit testing. Since this only affects
tests, there is no downstream impact.

Cc: vitaly.prosyak@amd.com
Cc: christian.koenig@amd.com
Cc: alexander.deucher@amd.com
Cc: jesse.zhang@amd.com
Cc: harry.wentland@amd.com
Cc: zbigniew.kempczynski@intel.com
Cc: kamil.konieczny@linux.intel.com
Cc: ryszard.knop@intel.com
Cc: lucas.demarchi@intel.com
Cc: katarzyna.piecielska@intel.com
Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
Reviewed-by: Vitaly Prosyak <vitaly.prosyak@amd.com>
Signed-off-by: Peter Senna Tschudin <peter.senna@linux.intel.com>

Peter Senna Tschudin authored 2 weeks ago and Kamil Konieczny committed 2 weeks ago

Adds a simple library for interacting with kmemleak and add
unit testing for the library. There are two modes intended to
integrate with igt_runner:
 - once: collect kmemleaks after all test completed
 - each: collect kmemleaks after eachb test completes

Cc: vitaly.prosyak@amd.com
Cc: christian.koenig@amd.com
Cc: alexander.deucher@amd.com
Cc: jesse.zhang@amd.com
Cc: harry.wentland@amd.com
Cc: zbigniew.kempczynski@intel.com
Cc: kamil.konieczny@linux.intel.com
Cc: ryszard.knop@intel.com
Cc: lucas.demarchi@intel.com
Cc: katarzyna.piecielska@intel.com
Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
Reviewed-by: Vitaly Prosyak <vitaly.prosyak@amd.com>
Signed-off-by: Peter Senna Tschudin <peter.senna@linux.intel.com>
Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

Peter Senna Tschudin authored 1 month ago and Kamil Konieczny committed 1 month ago

Commit ddfde25f ("runner: Add support for aborting on
network failure") introduced a 20 second deadline for the DUTs
network to recover after a suspend/resume cycle. If the network
isn't back up within that time, igt_runner aborts the test run
to save logs and prevent potential log loss from an imminent
power cycle.

External monitoring machine checks DUTs health and if it cannot
reach it over network for 60 seconds it will reboot DUT.

Although our CI system can be configured with a longer wait
time, extending it further would unnecessarily prolong tests in
cases of DUT hangs.

Bump the deadline to 40 seconds to prevent a premature igt_runner
abort.

Cc: Katarzyna Piecielska <katarzyna.piecielska@intel.com>
Cc: Ewelina Musial <ewelina.musial@intel.com>
Cc: Mateusz Grabski <mateusz.grabski@intel.com>
Cc: Konrad Brodzik <konrad.b.brodzik@intel.com>
Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Signed-off-by: Peter Senna Tschudin <peter.senna@linux.intel.com>
Acked-by: Ryszard Knop <ryszard.knop@intel.com>

For easier repro scenarios, add the cmdline to the json: one can see the
exact command executed to try to reproduce a CI failure without needing
extra files.

Adding cmdline to the results.json doesn't need a version upgrade:
piglit can still parse the file.

Tested-by: Peter Senna Tschudin <peter.senna@linux.intel.com>
Reviewed-by: Peter Senna Tschudin <peter.senna@linux.intel.com>
Acked-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Link: https://lore.kernel.org/r/20250207231039.2883195-12-lucas.demarchi@intel.com


Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

Serialize the command line to metadata.txt. The expected format in the
metadata.txt is like below:

	cmdline.argc : 6
	cmdline.argv[0] : ./build/runner/igt_runner
	cmdline.argv[1] : -o
	cmdline.argv[2] : --test-list
	cmdline.argv[3] : /tmp/testlist.txt
	cmdline.argv[4] : build/tests/
	cmdline.argv[5] : /tmp/results

Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>
Tested-by: Peter Senna Tschudin <peter.senna@linux.intel.com>
Reviewed-by: Peter Senna Tschudin <peter.senna@linux.intel.com>
Link: https://lore.kernel.org/r/20250207231039.2883195-11-lucas.demarchi@intel.com


Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

Prepare parser/serialize to handle arrays.

Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>
Tested-by: Peter Senna Tschudin <peter.senna@linux.intel.com>
Reviewed-by: Peter Senna Tschudin <peter.senna@linux.intel.com>
Link: https://lore.kernel.org/r/20250207231039.2883195-10-lucas.demarchi@intel.com


Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

Currently there's no support for newlines on arguments passed to runner.
However it's also a silent failure:

	# igt_runner --test-list '/tmp/test
	list2.txt' build/tests/ /tmp/results

	# head /tmp/results/metadata.txt
	disk_usage_limit : 0
	test_list : /tmp/test
	list2.txt
	name : results
	...

	# ./build/runner/igt_resume /tmp/results
	[9840425.334900] All tests already executed.
	resume failed at generating results
	Done.

Embedding a newline like this is very dubious for test-list, but it's
used for e.g. hooks. In future we will add the command line to the
metadata and possibly migrate the hooks, so add support for
escaping/unescaping the string on save/restore.

The method chosen is slightly different than the one used for hooks:
instead of adding a escape char and keeping the char escaped, this just
prefers using an hex representation of the char with a \x<HEX>h
sequence. This makes it easier when unescaping since the reader can
continue reading one line per iteration. In future this can also be
adopted by the hooks or even migrating the hooks to use metadata.txt.

Another fix is that now we just skip null values on the serialization
side.  Previously it would serialize "(null)" and then load that string
instead of NULL. Add code_coverage_script to the runner_test to cover
that, which would previously fail.

Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>
Tested-by: Peter Senna Tschudin <peter.senna@linux.intel.com>
Reviewed-by: Peter Senna Tschudin <peter.senna@linux.intel.com>
Link: https://lore.kernel.org/r/20250207231039.2883195-9-lucas.demarchi@intel.com


Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

No need to strdup() again since the fscanf() function is already
allocating the variable. Just set the pointer to NULL so we "leak" our
variable to be saved in the settings.

Tested-by: Peter Senna Tschudin <peter.senna@linux.intel.com>
Reviewed-by: Peter Senna Tschudin <peter.senna@linux.intel.com>
Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>
Link: https://lore.kernel.org/r/20250207231039.2883195-8-lucas.demarchi@intel.com


Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

Use similarly named macros on both sides of serialize/parse.

Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>
Tested-by: Peter Senna Tschudin <peter.senna@linux.intel.com>
Reviewed-by: Peter Senna Tschudin <peter.senna@linux.intel.com>
Link: https://lore.kernel.org/r/20250207231039.2883195-7-lucas.demarchi@intel.com


Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

Simplify assigning the variables by using functions called by wrapper
macros. This avoids calling atoi() on every iteration and will help
future refactors on functions parsing the values.

The pointer to the value is passed to the parse function since it will
be useful later when parsing a string and leaking it to the settings
struct rather than duplicating.

Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>
Tested-by: Peter Senna Tschudin <peter.senna@linux.intel.com>
Reviewed-by: Peter Senna Tschudin <peter.senna@linux.intel.com>
Link: https://lore.kernel.org/r/20250207231039.2883195-6-lucas.demarchi@intel.com


Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

Deduplicate cleanup so it's also easy to parse the line in a different
way.

Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>
Tested-by: Peter Senna Tschudin <peter.senna@linux.intel.com>
Reviewed-by: Peter Senna Tschudin <peter.senna@linux.intel.com>
Link: https://lore.kernel.org/r/20250207231039.2883195-5-lucas.demarchi@intel.com


Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

Keep valgrind happy with the normal allocations so we can find the real
leaks. Avoid this kind of leak:

	==806592== 8 bytes in 1 blocks are definitely lost in loss record 46 of 188
	==806592==    at 0x4846828: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
	==806592==    by 0x4BD534E: strdup (strdup.c:42)
	==806592==    by 0x10E6AA: parse_options (settings.c:863)
	==806592==    by 0x10D2DD: main (runner.c:19)

This only fixes the leaks on success. The error path handling on both
igt_resume and igt_runner will need some more work on how they are
interacting with all the initializations/cleanups.

Tested-by: Peter Senna Tschudin <peter.senna@linux.intel.com>
Reviewed-by: Peter Senna Tschudin <peter.senna@linux.intel.com>
Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>
Link: https://lore.kernel.org/r/20250207231039.2883195-4-lucas.demarchi@intel.com


Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

Make sure to free it when clearing settings.

Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>
Tested-by: Peter Senna Tschudin <peter.senna@linux.intel.com>
Reviewed-by: Peter Senna Tschudin <peter.senna@linux.intel.com>
Link: https://lore.kernel.org/r/20250207231039.2883195-3-lucas.demarchi@intel.com


Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

Peter Senna Tschudin authored 3 months ago and Kamil Konieczny committed 3 months ago

Modifies igt_runner to include calls to igt_facts() before the
execution of each test and after the final test concludes. Facts are
disabled by default, so add command line options to igt_runner to
enable facts:  -f, --facts

Updates serialize_settings() and read_settings_from_file() to save
and restore igt_runner settings to and from disk. This is used when
calling igt_runner with '--dry-run' and then by calling igt_resume
instead of igt_runner.

Updates unit testing for igt_runner to test that:
 - Facts are disabled by default
 - Facts can be enabled by command line arguments
 - The choice about facts being enabled or not is saved to disk
   and restored from disk

CC: Helen Koike <helen.koike@collabora.com>
CC: Jani Nikula <jani.nikula@linux.intel.com>
CC: Jani Saarinen <jani.saarinen@intel.com>
CC: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
CC: Juha-Pekka Heikkila <juha-pekka.heikkila@intel.com>
CC: Kamil Konieczny <kamil.konieczny@linux.intel.com>
CC: Lucas De Marchi <lucas.demarchi@intel.com>
CC: Maíra Canal <mcanal@igalia.com>
CC: Melissa Wen <mwen@igalia.com>
CC: Petri Latvala <adrinael@adrinael.net>
CC: Rob Clark <robdclark@chromium.org>
CC: Ryszard Knop <ryszard.knop@intel.com>
CC: Swati Sharma <swati2.sharma@intel.com>
CC: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
CC: dominik.karol.piatkowski@intel.com
CC: himal.prasad.ghimiray@intel.com
CC: katarzyna.piecielska@intel.com
CC: luciano.coelho@intel.com
CC: nirmoy.das@intel.com
CC: stuart.summers@intel.com
Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Reviewed-by: Ryszard Knop <ryszard.knop@intel.com>
Reviewed-by: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>
Signed-off-by: Peter Senna Tschudin <peter.senna@linux.intel.com>

Create a message before terminating a running test and
inform user what is a cause.

Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Reviewed-by: Jan Sokolowski <jan.sokolowski@intel.com>
Reviewed-by: Ryszard Knop <rk@dragonic.eu>

Instead of just returning and leaking memory and file descriptors
inform user about an error which occurred and terminate
gracefully.

Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Reviewed-by: Jan Sokolowski <jan.sokolowski@intel.com>
Reviewed-by: Ryszard Knop <rk@dragonic.eu>

There was no disk limit checks in reading kernel dmesg and that
could lead to writing really huge dumps longer than 400MB,
greatly exceeding disk limits used by CI and hardly useful for
developers. Make a dmesg dumping in chunks, size depending on
number of CPUs present, with a minimum of 64KB.
This could also allow to kick in disk limits checks if a driver
starts spilling messages into dmesg.

v2: correct size at last dump, also inform user about exceeding
  limits (Kamil)

Closes: #129


Cc: Petri Latvala <adrinael@adrinael.net>
Cc: Karol Krol <karol.krol@intel.com>
Cc: Ewelina Musial <ewelina.musial@intel.com>
Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Reviewed-by: Jan Sokolowski <jan.sokolowski@intel.com>
Reviewed-by: Ryszard Knop <rk@dragonic.eu>

Some messages generated by driver are triggered by test itself
and are not meant to rise error nor warn within runner, yet
they should be catched in other circumstances so they are not
suitable to be ignored permanently.

Instead of hard-coding such situations check dmesg for info from
a test and add such regex on a fly and then ignore next dmesg
errors or warns which match it.

Ignored regex will be removed after end of current subtest, just
before a new subtest or dynamic subtest starts.

v2: removed local copy for regex string (Zbigniew), clarify
 description, free regex before creating new (Kamil)
v3: print error when compiling regex fails (Zbigniew)
v4: restore cutting regex string at \n, it was lost in v2
 (Kamil)

Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

In testing drm-tip it is helpful to have reported all tests
results, especially when one have many dynamic ones we want
to get a parent result.

v2: fix for runner_tests (Jari)

Cc: Ewelina Musial <ewelina.musial@intel.com>
Cc: Helen Koike <helen.koike@collabora.com>
Cc: Jari Tahvanainen <jari.tahvanainen@intel.com>
Cc: Petri Latvala <adrinael@adrinael.net>
Cc: Rob Clark <robdclark@gmail.com>
Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Reviewed-by: Peter Senna Tschudin <peter.senna@intel.com>
Tested-by: Jari Tahvanainen <jari.tahvanainen@intel.com>

Gustavo Sousa authored 7 months ago and Matt Roper committed 7 months ago

Test binaries now allow passing multiple --hook options, it just makes
sense that igt_runner follows suit, so let's do it.

Note that this requires having another file in the results directory for
storing the hook strings, as metadata.txt does not support multivalued
items.

Since we are using a different file to store hook strings, take this
opportunity to also allow multiline hook strings (which was not possible
with metadata.txt).

Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
Link: https://lore.kernel.org/r/20240814204822.95283-7-gustavo.sousa@intel.com


Signed-off-by: Matt Roper <matthew.d.roper@intel.com>

Gustavo Sousa authored 7 months ago and Matt Roper committed 7 months ago

Extend the current hook functionality to allow using multiple hook
descriptors. That allows running a test binary like the following:

    my-test --hook pre-subtest:do-something \
            --hook post-subtest:do-somthing-else

Which is more convenient to the user than having to implement a script
that checks the value of IGT_HOOK_EVENT environment variable.

Note that we still need to add the same support for igt_runner, which is
left for a followup change.

Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
Link: https://lore.kernel.org/r/20240814204822.95283-6-gustavo.sousa@intel.com


Signed-off-by: Matt Roper <matthew.d.roper@intel.com>

Gustavo Sousa authored 7 months ago and Matt Roper committed 7 months ago

Now that we have support for setting a hook script for test cases, let's
also add the option --hook to igt_runner, which forwards it to test
executables.

Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
Link: https://lore.kernel.org/r/20240814204822.95283-4-gustavo.sousa@intel.com


Signed-off-by: Matt Roper <matthew.d.roper@intel.com>

Gustavo Sousa authored 7 months ago and Matt Roper committed 7 months ago

In an upcoming change, we will be adding the option to forward the
--hook option to the test cases, which will require updating
execute_test_process() to add the option when asked by the user.

The current implementation makes that task not quite straightforward:
filling of argv is already dependent on stuff like entry->subtest_count
and dynbegin; if we want to keep on using constant indices, we would
need several conditional branches for adding arguments for --hook.

Let us change the current implementation to use a dynamic vector, to
make it easier to extend argv with more stuff as needed.

v2:
  - Squash the logic from patch "runner: Use dynamic vector for test
    argv" directly instead of using the original logic, which used a
    statically sized array. (Lucas)

Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
Link: https://lore.kernel.org/r/20240814204822.95283-3-gustavo.sousa@intel.com


Signed-off-by: Matt Roper <matthew.d.roper@intel.com>

Gustavo Sousa authored 10 months ago and Matt Roper committed 10 months ago

The function job_list_from_test_list() uses a while loop to read entries
from the testlist.

The condition ((delim = strchr(binary, '@')) != NULL) checks if the
current entry specifies a subtest. When no subtest is specified, the
else clause will cause a jump to the next iteration. That means that we
are certain any statement executed after that if block has (delim !=
NULL). As such, all null checks on that variable after that point are
pointless.

In fact, certain unnecessary null checks might even cause confusion to
readers. One example is the block meant to display the "Unexpected test
without subtests ..." message: it would never happen, since that
condition is handled earlier in the iteration (i.e. the full set of
subtests is expanded for that entry in the else clause previously
mentioned). The following execution (done before this change)
illustrates that:

    $ cat <<EOF > /tmp/foo.testlist
    igt@xe_pm@s2idle-basic
    igt@xe_pm
    igt@xe_pm@s2idle-exec-after
    EOF

    $ ./build/runner/igt_runner -d -m --test-list /tmp/foo.testlist build/tests /tmp/results
    [36408.109043] Dry run, not executing. Invoke igt_resume if you want to execute.
    Done.

    $ cat /tmp/results/joblist.txt | sed 's/^\(.\{50\}\).\+/\1.../'
    xe_pm s2idle-basic
    xe_pm s2idle-basic,s2idle-basic-exec,s2idle-exec-a...
    xe_pm s2idle-exec-after

Let's remove those unnecessary checks.

Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
Reviewed-by: Ngai-Mint Kwan <ngai-mint.kwan@linux.intel.com>
Link: https://lore.kernel.org/r/20240430211934.349775-2-gustavo.sousa@intel.com


Signed-off-by: Matt Roper <matthew.d.roper@intel.com>

Reagan Bohan authored 10 months ago and Kamil Konieczny committed 10 months ago

sys/poll.h is non-standard and including it on musl produces a warning.

Signed-off-by: Reagan Bohan <reagan@ourmail.work>
Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

Piotr Kira authored 1 year ago and Kamil Konieczny committed 1 year ago

Runtimes for single subtest are not included in results json object
after switching to socket communication. Make times for both subtests
and dynamic subtests when using comms.

v2: added GitLab issue number, added Petri to Cc (Kamil)
  reformat so it could be applied (Kamil)

Cc: Petri Latvala <adrinael@adrinael.net>
Fixes: 9d889fa6 ("runner: Use socket communications")
Closes: drm/igt-gpu-tools#132


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

As regular expressions are now case-insensitive, add ab
unit test to verify that case-insensitive logic is working
as expected.

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

There are some KMS tests that are case-sensitive. Eventually,
those could be blocklisted with a different case, causing troubles
for CI runs.

As it makes no sense to have the same test name with different
cases, handle regular expressions in a case-insensitive way.

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

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>