public inbox for igt-dev@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: igt-dev@lists.freedesktop.org, Petri Latvala <petri.latvala@intel.com>
Subject: Re: [igt-dev] [PATCH i-g-t 0/6] Dynamic subtests
Date: Fri, 21 Jun 2019 10:26:31 +0100	[thread overview]
Message-ID: <e93a9f50-db05-7738-fb86-30202367cb2d@linux.intel.com> (raw)
In-Reply-To: <20190620080959.GB22949@platvala-desk.ger.corp.intel.com>


On 20/06/2019 09:09, Petri Latvala wrote:
> On Thu, Jun 20, 2019 at 08:47:12AM +0100, Tvrtko Ursulin wrote:
>>
>> On 19/06/2019 12:51, Petri Latvala wrote:
>>> Have you ever had a massive list of named things that you'd quite like
>>> to test with separate subtests but maintaining a compile-time
>>> exhaustive list of those things seems impossible, stupid, or both?
>>>
>>> Have you ever wished you could just go over a list of things at
>>> runtime and create subtests on the fly?
>>>
>>> Have you ever looked at a long list of SKIP-results and wondered if it
>>> makes sense to keep plopping those in CI results?
>>>
>>> If so, this is your lucky day, for dynamic subtests are here!
>>>
>>> While the restriction remains that "execution entry points" must still
>>> be statically available no matter what the runtime environment and
>>> configuration is, dynamic subtest containers are special subtest-like
>>> entry points that can contain dynamic subtests. For example:
>>>
>>> With normal subtests:
>>> for_each_pipe_static(pipe)
>>>     igt_subtest_f("fudge-with-pipe-%s", kmstest_pipe_name(pipe)) {
>>>       int fd = drm_open_driver(DRIVER_ANY);
>>>       igt_require(actually_have_this_pipe(fd, pipe));
>>>       igt_assert(do_stuff(fd, pipe));
>>>     }
>>>
>>>
>>> With dynamic subtests:
>>> igt_subtest_container("fudge")
>>>     int fd = drm_open_driver(DRIVER_ANY);
>>>     for_each_pipe(fd, pipe) {
>>>       igt_dynamic_subtest_f("pipe-%s", kmstest_pipe_name(pipe)) {
>>>         igt_assert(do_stuff(fd, pipe));
>>>       }
>>>     }
>>>
>>
>> How would the below be converted:
>>
>> int fd = drm_open_driver(DRIVER_ANY);
>>
>> for_each_pipe_static(pipe) {
>>    igt_subtest_group {
>>      igt_fixture {
>>        igt_require(actually_have_this_pipe(fd, pipe));
>>      }	
>>
>>      igt_subtest_f("fudge-with-pipe-%s", pipe_name(pipe))
>>        fudge(fd, pipe);
>>
>>      igt_subtest_f("kludge-with-pipe-%s", pipe_name(pipe))
>>        kludge(fd, pipe);
>>    }
>> }
>>
> 
> 
> igt_dynamic_subtest_container("fudge")
>    for_each_pipe(fd, pipe)
>      igt_dynamic_subtest_f("pipe-%s", kmstest_pipe_name(pipe))
>        fudge(fd, pipe);
> 
> igt_dynamic_subtest_container("kludge")
>    for_each_pipe(fd, pipe)
>      igt_dynamic_subtest_f("pipe-%s", kmstest_pipe_name(pipe))
>        kludge(fd, pipe);
> 
> 
> 
> Then you get results for "subtests" called igt@binaryname@fudge and
> igt@binaryname@fudge@pipe-a and igt@binaryname@fudge@pipe-b, along
> with igt@binaryname@kludge and igt@binaryname@kludge@pipe-a and
> igt@binaryname@kludge@pipe-b
> 
> 
> 
>> ?
>>
>>>
>>> Running on hardware that only has pipes A-C? pipe-d doesn't even get
>>> attempted.
>>>
>>>
>>> Semantics
>>> =========
>>>
>>> Dynamic subtests can only appear in a subtest container. The name is
>>> up to debate: This series calls it igt_subtest_container().
>>
>> igt_dynamic_subtest_group?
>>
>>> A dynamic subtest container can only contain dynamic subtests, it
>>> cannot do failures on its own. In other words:
>>>    - igt_assert not allowed
>>>    - igt_skip / igt_require is allowed though
>>
>> Can it do fixtures?
> 
> No.
> 
> For shared initialization, do
> 
> igt_dynamic_subtest_container("hello-init") {
>    /* right here */
>    int fd = drm_open_driver(DRIVER_ANY);
>    init_stuffs(fd);
>    /* init done */
> 
>    for (......) {
>      igt_dynamic_subtest(...)
> }
> 
> 
>>
>>> Any failing dynamic subtest will result in the container reporting a
>>> failure.
>>>
>>> Not executing any dynamic subtests from a container will result in the
>>> container reporting a SKIP automatically. Best practices: If possible,
>>> instead of using igt_require in an igt_dynamic_subtest, just don't
>>> enter it for such an occasion. In other words:
>>>
>>> Do not:
>>> for_each_pipe(fd, pipe)
>>>     igt_dynamic_subtest("%s", kmstest_pipe_name(pipe)) {
>>>       igt_require(pipe_has_things(pipe));
>>>       ...
>>>     }
>>>
>>>
>>> Instead do:
>>> for_each_pipe(fd, pipe) {
>>>     if (!pipe_has_things(pipe))
>>>       continue;
>>>     igt_dynamic_subtest("%s", kmstest_pipe_name(pipe)) {
>>>       ...
>>>     }
>>> }
>>>
>>> That way, tests that currently carefully track the number of, say,
>>> connected displays of type $x, to properly skip when their amount is
>>> 0, will get their SKIP automatically instead.
>>>
>>> Dynamic subtests are filterable: Just like --run-subtest, there's
>>> --dynamic-subtest that takes glob expressions. This is for debugging
>>> only, CI will not use them.
>>>
>>> Dynamic subtests are _NOT_ listable. While it might be useful,
>>> implementing listing requires another layer of igt_fixture usage
>>> inside dynamic subtest containers and I'd rather have the code be
>>> simple than have listing. The default of running all dynamic subtests
>>> should make sense for most cases, and special cases (like debugging)
>>> should be able to know already what they want to run.
>>>
>>>
>>> Results in CI: CI will show results for both the container, and the
>>> dynamic subtests within it. The naming is:
>>>
>>> igt@binary@subcontainer  -  the container, has as its output the whole
>>> shebang that dynamic subtests printed.
>>>
>>> igt@binary@subcontainer@dynamicname  -  a dynamic subtest appears as a
>>> separate name, WITHOUT grouping or nesting. Any relation to the
>>> container will not be linked at this point. Possibly in the future
>>> when a usability wizard figures out the best way to browse results...
>>
>> Did not get this part. Is it the first or second option? Or both? If it is
>> the second then CI has to handle dynamic subtests so why we even need the
>> complication to start with?
> 
> 
> It's both. The container will report a result (based on what the
> dynamic tests did) and there's also a result for each dynamic subtest.
> 
> The complication is needed because with dynamic subtests the execution
> entry and results reporting are two different sets, reported subtests
> being a superset of the entries.
> 
> CI will handle (knocking on wood here) dynamic results fine in its
> current state, because dynamic subtest results walk like a duck, smell
> like a duck, and look like normal subtest results.

I am confused by all this. Could we then achieve the same result by just 
adding the ability to hide subtests from test enumeration?

igt_subtest_group {
	igt_subtest("a");
}

igt_dynamic_subtest_group("group") {
	igt_subtest("b");
}

# ./test --list-subtests
a
group

# .test --list-all-subtests
a
group-b

# ./test --r group
PASS

# ./test --r group-b
PASS

Would the above satisfy CI requirements? Sounds like it would be easier 
to implement wrt needed code base changes in individual tests and 
possibly the IGT core code would not be difficult either? Just needs to 
track in what kind of subgroup it is to decide what to do when 
listing/running tests.

Regards,

Tvrtko

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

  reply	other threads:[~2019-06-21  9:26 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-19 11:51 [igt-dev] [PATCH i-g-t 0/6] Dynamic subtests Petri Latvala
2019-06-19 11:51 ` [igt-dev] [PATCH i-g-t 1/6] lib: Introduce dynamic subtests Petri Latvala
2019-06-19 11:51 ` [igt-dev] [PATCH i-g-t 2/6] lib/tests: Unit tests for " Petri Latvala
2019-06-19 11:51 ` [igt-dev] [PATCH i-g-t 3/6] runner/resultgen: Refactor output parsing Petri Latvala
2019-06-19 11:51 ` [igt-dev] [PATCH i-g-t 4/6] runner/json_tests: Adapt to better " Petri Latvala
2019-06-19 11:51 ` [igt-dev] [PATCH i-g-t 5/6] runner: Parse dynamic subtest outputs and results Petri Latvala
2019-06-19 11:51 ` [igt-dev] [PATCH i-g-t 6/6] runner/json_tests: Test dynamic subtests Petri Latvala
2019-06-19 15:03 ` [igt-dev] ✓ Fi.CI.BAT: success for Dynamic subtests Patchwork
2019-06-20  7:47 ` [igt-dev] [PATCH i-g-t 0/6] " Tvrtko Ursulin
2019-06-20  8:09   ` Petri Latvala
2019-06-21  9:26     ` Tvrtko Ursulin [this message]
2019-07-22 12:32       ` Petri Latvala
2019-08-02 14:23         ` Daniel Vetter
2019-06-20  9:06 ` [igt-dev] ✓ Fi.CI.IGT: success for " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e93a9f50-db05-7738-fb86-30202367cb2d@linux.intel.com \
    --to=tvrtko.ursulin@linux.intel.com \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=petri.latvala@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox