Re: [PATCH] tests: ddx_intel_after_fbdev loads intel ddx after fbdev was loaded.

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Aug 21, 2013 at 1:55 PM, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:
> On Wed, Aug 21, 2013 at 6:27 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
>> On Wed, Aug 21, 2013 at 11:16 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>>> On Wed, Aug 21, 2013 at 11:00:53AM +0200, Daniel Vetter wrote:
>>>> On Tue, Aug 20, 2013 at 03:43:05PM -0300, Rodrigo Vivi wrote:
>>>> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
>>>>
>>>> I'm confused what exactly this tests, since it seems to never fail ...
>
> fb.o #68030
>
>>>> automated tests should have some checks for expected results.
>>>>
>>>> Also I'm not sure whether we want such ddx/X tests in igt ...
>
> actually it was your idea and I liked it ;)

Ah, what I meant was a reduced testcase to replay just the special
modesetting sequence to reproduce the bug. Similar to what Imre recently
has done with the kms_setmode.c testcase. If we add the entire script we
essentially depend upon fbdev and our own ddx to not change behaviour ...

>>> Whether or not it makes a good test, it is nice to have a repository of
>>> the little hacks we use for debugging. From little acorns mighty oaks
>>> grow.
>>
>> Agreed, but then it imo shouldn't be added to the default list of
>> targets of tests to run.
>
> Agreed. tbh I didn't realized I was doing that by adding it to TESTS_scripts
>
>> We already have a bunch of these scripts
>> added to EXTRA_DIST, I guess adding a new variable SCRIPTS would be
>> good.
>
> Do you think we need an extra directory for scripts like this? or just
> create this new SCRIPTS variable at Makefile.am?

Since it's essentially a special testcase script I think tests/ is good
enough. Something like the below diff:

Cheers, Daniel

diff --git a/tests/Makefile.am b/tests/Makefile.am
index ddb709f..805d90f 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -170,8 +170,14 @@ HANG = \
 	gem_non_secure_batch \
 	$(NULL)
 
+scripts = \
+	  check_drm_clients \
+	  debugfs_wedged\
+	  drm_lib.sh \
+	  $(NULL)
+
 EXTRA_PROGRAMS = $(TESTS_progs) $(TESTS_progs_M) $(HANG)
-EXTRA_DIST = $(TESTS_scripts) $(TESTS_scripts_M) drm_lib.sh check_drm_clients debugfs_wedged
+EXTRA_DIST = $(TESTS_scripts) $(TESTS_scripts_M) $(scripts)
 CLEANFILES = $(EXTRA_PROGRAMS)
 
 AM_CFLAGS = $(DRM_CFLAGS) $(CWARNFLAGS) \
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch