Re: [PATCH v2] kselftest: alsa: Add simplistic test for ALSA mixer controls kselftest

From: "Takashi Sakamoto" <o-takashi@sakamocchi.jp>
To: "Mark Brown" <broonie@kernel.org>, "Takashi Iwai" <tiwai@suse.de>,
	"Shuah Khan" <shuah@kernel.org>,
	"Jaroslav Kysela" <perex@perex.cz>,
	alsa-devel@alsa-project.org, linux-kselftest@vger.kernel.org
Subject: Re: [PATCH v2] kselftest: alsa: Add simplistic test for ALSA mixer controls kselftest
Date: Wed, 08 Dec 2021 23:26:59 +0900	[thread overview]
Message-ID: <85e0af0d-6f7e-47b0-b09c-c6006ee50662@www.fastmail.com> (raw)
In-Reply-To: <Ya9uvPx37AcOdwLK@sirena.org.uk>

On Tue, Dec 7, 2021, at 23:25, Mark Brown wrote:
> On Tue, Dec 07, 2021 at 12:20:32PM +0900, Takashi Sakamoto wrote:
>> On Mon, Dec 06, 2021 at 04:03:05PM +0000, Mark Brown wrote:
>
>> I think it safer to take care of volatile attribute when comparing read
>> value to written value. I'm glad if you review below patch.
>
> Yes, that's a good spot, it was an oversight to not take care of
> volatile controls - I'll roll that in if I send a new version or
> I guess Takashi could apply on top of my v2?  If people are mostly happy
> and at Jaroslav is also preparing patches on top of this it might make
> sense to get it into git sooner.
>
> Reviewed-by: Mark Brown <broonie@kernel.org>

Feel free to append it for your new version with or without my sign-off.
If keeping it, I could review your respun one by receiving according to To
or Cc.

>> As another topic, the runtime of alsa-lib application largely differs
>> between process user due to the result of parsing text files for
>> configuration space. I can easily imagine that developers unfamiliar to
>> alsa-lib carelessly adds invalid or inadequate configurations to files
>> under target path of alsa-lib configuration space, and they are puzzled
>> since they are unaware of the fact that the kselftest is affected by
>> userspace stuffs for the runtime.
>
>> If we respect the basic theory of test (idempotence), we can use ioctl(2)
>> with requests for ALSA control interface since it's not so complicated
>> (at least it is easier than ALSA PCM interface). The purpose of
>> kselftest is to test kernel stuffs, not to test userspace stuffs
>> including alsa-lib implementation and variety of plugins.
>
> Right, I was originally thinking of implementing this in terms of
> tinyalsa which is much more direct (though I was amused to see that's
> gained userspace plugins at some point!) partly for this reason but the
> lack of widespread packaging for it was a bit of a blocker and it didn't
> feel like a great idea to essentially do yet another userspace ALSA
> library even if as you say it can be pretty trivial.  Jaroslav's
> suggestion of using a custom configuration to override the default seems
> like it addresses everything though.
>
> I do think there's an advantage for test comprehensibility in having the
> test written in terms of similar APIs to a normal userspace application
> - it makes it easier to relate what the test is doing to normal usage
> which is helpful when trying to understand what the test is trying to
> tell you.

In my opinion, test is merely test. It's not a sample program.

What important is what is tested. and how to assist developers if failed.
If more suitable for the direction, we should do it, even if using raw ioctl
in the case.

For your information, `check_event()` in `test/user-ctl-element-set.c`, my
rough implementation of test for event triggered by tlv operation, might
be helpful to you or start point t to discuss about event check.

Regards

Takashi Sakamoto