Re: [LTP] [PATCH 2/6] lib: Add .max_runtime and tst_remaining_runtime()

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> > This is another attempt of decoupling test runtime from timeouts.
> 
> Do we have documentation explaining what runtimes and timeouts are?
> 
> These two words are interchangeable.

Not yet I plan to rewrite documentation once things are finalized
enough.

> > Fundamentally there are two types of tests in LTP. First type are tests
> > that are rather quick (much less than a second) and can live with
> > whatever default timeout we set up. Second type of tests are tests that
> > run in a loop until timeout or a number of iterations is reached, these
> > are the tests that are going to be converted to the .max_runtime added
> > by this patch and followups.
> 
> The code looks good, but this all feels quite hard to parse on a
> more abstract level. I think it is because you are trying to over
> generalise between different categories of test.
> 
> So we have tests which run to completion as fast as they can. Then we
> have tests which iterate for some arbitrary time period (usually capped
> by some number of iterations as well, but it's not important).
> 
> In the first case the concept of a target runtime makes no sense. So we
> end up with 'max runtime' which is the same type of thing as the timeout
> (although the value of timeout is greater).
>
> In the second case, where the test tries to execute for some length of
> time. Then the target runtime and timeout are really seperate things.
> 
> I would suggest renaming 'max_runtime' to just 'runtime' and make it
> optional (defaults to 0). All tests which are of the second type can set
> .runtime = DEFAULT_RUNTIME (or whatever). If the test tries to use
> tst_remaining_runtime() without runtime being set, then an error is
> thrown.

That more of less what the patchset attempts to do. It hardcodes the
runtime as a number instead of having DEFAULT_RUNTIME constant though.

> If runtime is present then it can simply be added to the timeout to
> produce the "total timeout" (total_timeout = runtime + timeout).

I guess that would work reasonably well.

> This has the advantage of clearly showing in the meta data which tests
> are likely to run for some time. Also it means that the concept of
> 'runtime' doesn't change depending on the type of test. Finally we can
> set timeout very low by default.
> 
> So when calculating how long a test executable may run for we have
> 
> (runtime + timeout) * variants * iterations
>
> The wording is still not great. runtime is more like "deliberate
> runtime" and timeout is "maximal expected runtime after the deliberate
> runtime".

Sounds good. I will work on v2 that would include these changes.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp