All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: Mauro Carvalho Chehab <mauro.chehab@linux.intel.com>
Cc: igt-dev@lists.freedesktop.org
Subject: Re: [igt-dev] [PATCH i-g-t 2/2] testplan/meson.build: make it check for missing i915 documentation
Date: Tue, 4 Jul 2023 14:03:59 +0100	[thread overview]
Message-ID: <b197f870-0e9d-37b0-9b58-0d2f77325480@linux.intel.com> (raw)
In-Reply-To: <20230704145053.31c39a24@maurocar-mobl2>


On 04/07/2023 13:50, Mauro Carvalho Chehab wrote:
> On Tue, 4 Jul 2023 13:41:14 +0100
> Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> 
>> On 04/07/2023 13:28, Tvrtko Ursulin wrote:
>>>
>>> On 26/05/2023 07:46, Mauro Carvalho Chehab wrote:
>>>> From: Mauro Carvalho Chehab <mchehab@kernel.org>
>>>>
>>>> Now that i915 is fully documented, check it at build time.
>>>
>>> This step seems to be slow as molasses and it also rebuilds the Xe test
>>> plan when I touch an i915 test.
> 
> This is fixable, but better to wait for Bhanu's patch series that will
> be moving the Intel tests to a new directory (tests/intel/).
> 
>>>
>>> What is the way to disable it all when configuring the build?
> 
> Yes, you can disable it:
> 
> 	$ meson -Dtestplan=disabled build --reconfigure

Works, thanks!

> We do want this enabled by default, as CI needs to check it and
> reject patches that aren't updating tests documentation.
> 
> Our internal CI is already dependent on it for the Xe and KMS, and
> the plan is to extend it to i915 as well, to get rid of lots of
> hacks that currently maps tests with the tested features.

As long as the build process is not smart enough to only check a single 
modified test, FWIW disabled by default sounds better to me and CI can 
easily enable it.

>>> P.S. I also find the "now that i915 is fully documented" statement a bit
>>> of a chuckle, since random two tests I happened to open haven't really
>>> been documented - it rather looks to be a bit of a charade.
> 
> Well, it is as good as what we had documented on IGT itself and on
> some separate spreadsheets. If you find anything odd, please fix it.

I happened to open i915_pm_rps yesterday and drm_fdinfo today. Majority 
of documentation are just place holders to cheat the verification step. 
Similarly I don't think it will be "enforceable" during code review and 
such silliness will just land. Shrug. sometimes even best intentions 
don't lead where you'd expect them to.

>>> I wouldn't care really apart from it significantly slowing down the
>>> development workflow.
>>
>> # time ninja
>> [1/448] Generating lib/version.h with a custom command
>> fatal: not a git repository (or any of the parent directories): .git
>> [6/6] Generating docs/testplan/i915_tests.rst with a custom command
>>
>> real    0m24.363s
>> user    0m6.530s
>> sys     0m20.968s
>>
>> 24 seconds.. I just changed one i915 test. :(
> 
> What it takes time is not building the docs, but to run all tests with
> "--list" parameter, in order to double-check if every test has some
> documentation.

I don't really care what takes time, just that it was unbearable. But 
now you gave me a workaround so that's good enough for me.

Regards,

Tvrtko


  reply	other threads:[~2023-07-04 13:04 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-26  6:46 [igt-dev] [PATCH i-g-t 0/2] Enable documentation check for i915 driver Mauro Carvalho Chehab
2023-05-26  6:46 ` [igt-dev] [PATCH i-g-t 1/2] i915/perf_pmu: sync documentation with latest changes Mauro Carvalho Chehab
2023-05-26 12:25   ` Kamil Konieczny
2023-05-26  6:46 ` [igt-dev] [PATCH i-g-t 2/2] testplan/meson.build: make it check for missing i915 documentation Mauro Carvalho Chehab
2023-05-26 12:26   ` Kamil Konieczny
2023-07-04 12:28   ` Tvrtko Ursulin
2023-07-04 12:41     ` Tvrtko Ursulin
2023-07-04 12:50       ` Mauro Carvalho Chehab
2023-07-04 13:03         ` Tvrtko Ursulin [this message]
2023-07-05  8:56           ` Mauro Carvalho Chehab
2023-07-05 15:33             ` Mauro Carvalho Chehab
2023-05-26 13:04 ` [igt-dev] ✓ Fi.CI.BAT: success for Enable documentation check for i915 driver Patchwork
2023-05-27  4:54 ` [igt-dev] ✓ Fi.CI.IGT: " 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=b197f870-0e9d-37b0-9b58-0d2f77325480@linux.intel.com \
    --to=tvrtko.ursulin@linux.intel.com \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=mauro.chehab@linux.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.