Re: [RFC i-g-t] tests/gem_exec_basic: Documentation for subtests

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 8/10/2017 1:32 AM, Arkadiusz Hiler wrote:
On Wed, Aug 09, 2017 at 10:00:16AM -0700, Belgaumkar, Vinay wrote:

On 8/9/2017 7:32 AM, Arkadiusz Hiler wrote:
On Tue, Aug 08, 2017 at 03:09:00PM -0700, Vinay Belgaumkar wrote:
This is an RFC for adding documentation to IGT subtests. Each subtest can have
something similar to a WHAT - explaining what the subtest actually does,
and a WHY - which explains a use case, if applicable. Additionally,
include comments for anything in the subtest code which can help
explain HOW the test has been implemented. We don't actually need the WHAT
and WHY tags in the documentation.

These comments will not be linked to gtkdoc as of now, since we do not have a
   mechanism to link it to every subtest name.
Hey Vinay,

I get similar feelings towards this RFC as Lukasz and Radek do.

Was your intention to propose format of the comments? Or maybe force
people to comment more on the code? Or just pointing out that we could
use some subtest documentation?

You are not documenting subtests, you are documenting arbitrary
functions that may or may not be used as a subtest.

I cannot help but feel lost here.

Being explicit as of your intention and coming up with more abstract or
better examples would also help, as current ones are detracting from the
idea itself.

I do not get this RFC and it's purpose but I am looking forward to
seeing revised version that is clearer on your intentions and easier to
grasp.

Hi Arek,
      The purpose of this RFC is to complement Petri's subtest documentation
patch. That patch will give us
an ability to add a line of documentation per subtest, which is definitely
useful. However, what I noticed is
that when you actually start debugging a test issue and step into the
subtest code, it is very hard to understand
what the purpose of certain commands are. My intention was to provide a text
only documentation in the test source
to allow test developers to understand the code better. It's hard to explain
the how and and why all in a single sentence.

If we provided an ability/guideline to test developers for mentioning the
same at the beginning of the actual subtest code,
The ability is already there - the comments.
What we need is the drive to do the commenting.

IMO the issue is that one thing may be obvious to some and at the same
time look arcane to others - people are generally biased and treat
themselves as the baseline.

This is not solvable by guideline or a RFC imposing a commenting format.

This should be a joint effort:

  * authors trying to be more empathetic - hard to do and impossible to
    enforce

  * reviewers should point out things that are hard to understand and
    ask for clarification, preferably in a form of a slight rewrite of
    the code so it's more manageable, or as a comment if the former is not
    possible

  * people debugging such arcane parts (or asked the author) should take
    the effort and either make it more human readable or provide such
    commentary to save time of those who will come there after them

In case you haven't noticed - recently we started requiring that all
commits go through the mailing list, so everything is reviewed.

Feel free to participate in the discussion. It's okay even if you ask
for clarifications post-merge. They can be done in follow-up series, and
those don't need to be done by the original author.

As of guideline, having Petri's approach would already help a lot. I
would prefer to see how it does, and after it proves (or not) take
the further steps.

Totally agree with all the above points. I am fine with the approach of using a combination of Petri's subtest documentation function and adding any extra comments where necessary. The test code will get much easier to understand as long as we answer the WHAT, WHY and to some extent, the HOW of the code. The guideline I was proposing was merely a way to
achieve all of the above, we do not need a stringent format as such.


it can make debugging a lot more simpler rather than having to jump back to
the main function to figure out what the subtest
is supposed to do. I have also included some comments inside the test
function to explain why we use certain system calls.
Idea was not to define the system call, but explain why it is being used.
Since you are "debugging" the subtest  I don't think it's this much of
an effort to do --list-subtest-with-doc or do jump in the code once or
twice.

IMO it's better to have the information provided only once. It's already
hard to keep doc/comments up to date not dealing with such redundancy.

Again, I did not mean to duplicate the subtest documentation effort. My
initial plan was to send out an
RFC using Petri's patch as well, so that the intention is more clear. I can
do that with the second version of the patch if needed.
However, the main aim is to agree on a convention to add more documentation
to the subtest code so that it simplifies
debugging and helps with understanding of the aim behind writing the
particular subtest.
Waiting for v2 then :-)


_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux