[igt-dev] [PATCH i-g-t] igt/drv_module_reload: Don't reload on exit

[Petri Latvala]

On Fri, Jul 27, 2018 at 09:06:57AM +0100, Chris Wilson wrote:
> The next test will happily load whatever module it requires.
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  tests/drv_module_reload.c | 23 +++++++----------------
>  1 file changed, 7 insertions(+), 16 deletions(-)


I was going to comment that leaving a driver loaded with whatever
errorly parameters the tests use is bad... But you changed the
no-display subtest to also ensure the driver is unloaded when it's
done. I tend to forget conventions like that easily, can we have
comments in there stating that subtests should make sure to leave the
driver in either a valid reloaded state or unloaded?


> 
> diff --git a/tests/drv_module_reload.c b/tests/drv_module_reload.c
> index 34f55eab1..4c06e4caf 100644
> --- a/tests/drv_module_reload.c
> +++ b/tests/drv_module_reload.c
> @@ -340,21 +340,21 @@ hda_dynamic_debug(bool enable)
>  
>  igt_main
>  {
> -	int err;
> -
> -	igt_fixture
> +	igt_subtest("basic-reload") {
>  		hda_dynamic_debug(true);
>  
> -	igt_subtest("basic-reload") {
> -		if ((err = reload(NULL)))
> -			igt_fail(err);
> +		igt_assert_eq(reload(NULL), 0);
>  
>  		gem_sanitycheck();
>  		gem_exec_store();
> +
> +		hda_dynamic_debug(false);
>  	}


hda_dynamic_debug is no longer active for other subtests then. Is it
not useful for them?




>  
> -	igt_subtest("basic-no-display")
> +	igt_subtest("basic-no-display") {
>  		igt_assert_eq(reload("disable_display=1"), 0);
> +		igt_i915_driver_unload();
> +	}
>  
>  	igt_subtest("basic-reload-inject") {
>  		int i = 0;
> @@ -367,13 +367,4 @@ igt_main
>  		/* We expect to hit at least one fault! */
>  		igt_assert(i > 1);


Tautological assert here btw.



-- 
Petri Latvala