Hi, On Thu, Oct 26, 2023 at 03:06:39PM +0800, David Gow wrote: > On Wed, 20 Sept 2023 at 16:49, Maxime Ripard <mripard@xxxxxxxxxx> wrote: > > > > Kunit recently gained support to setup attributes, the first one being > > the speed of a given test, then allowing to filter out slow tests. > > > > A slow test is defined in the documentation as taking more than one > > second. There's an another speed attribute called "super slow" but whose > > definition is less clear. > > > > Add support to the test runner to check the test execution time, and > > report tests that should be marked as slow but aren't. > > > > Signed-off-by: Maxime Ripard <mripard@xxxxxxxxxx> > > > > --- > > > > To: Brendan Higgins <brendan.higgins@xxxxxxxxx> > > To: David Gow <davidgow@xxxxxxxxxx> > > Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> > > Cc: Rae Moar <rmoar@xxxxxxxxxx> > > Cc: linux-kselftest@xxxxxxxxxxxxxxx > > Cc: kunit-dev@xxxxxxxxxxxxxxxx > > Cc: linux-kernel@xxxxxxxxxxxxxxx > > > > Changes from v1: > > - Split the patch out of the series > > - Change to trigger the warning only if the runtime is twice the > > threshold (Jani, Rae) > > - Split the speed check into a separate function (Rae) > > - Link: https://lore.kernel.org/all/20230911-kms-slow-tests-v1-0-d3800a69a1a1@xxxxxxxxxx/ > > --- > > I quite like this, though agree somewhat with Rae's comments below. > > I personally think the time thresholds are, by necessity, very > 'fuzzy', due to the varying speeds of different hardware. Fortunately, > the actual runtime of tests seems pretty well stratified, so the exact > threshold doesn't really matter much. > > I ran some tests here, and all of the tests currently not marked slow > take <1s on every machine I tried (including the ancient 66MHz 486), > except for the drm_mm_* ones (which takes ~6s on my laptop, and times > out after 15 minutes on the aforementioned 486). Both the 1s and 2s > timeouts successfully distinguish those cases. I had a similar experience running the tests in qemu on a Pi4, which is probably the slowest machine we can reasonably expect. > Ideally, I think we'd have something like: > #define KUNIT_SPEED_SLOW_THRESHOLD_S 1 /* 1 sec threshold for 'slow' tests */ > #define KUNIT_SPEED_WARNING_MULTIPLIER 2 /* Warn when a test takes > > twice the threshold. */ > #define KUNIT_SPEED_SLOW_WARNING_THRESHOLD_S > (KUNIT_SPEED_WARNING_MULTIPLIER * KUNIT_SPEED_SLOW_THRESHOLD_S) > > Which is perhaps excessively verbose, but is very clear as to what > we're doing. It also gives more scope to allow the ratio to be > configured for people with very slow / fast machines in the future. > > Thoughts? That looks like a good compromise to me, I'll send another version :) Thanks! Maxime
Attachment:
signature.asc
Description: PGP signature
