Messages in this thread | | | Date | Thu, 26 Oct 2023 10:44:21 +0200 | From | Maxime Ripard <> | Subject | Re: [PATCH v2] kunit: Warn if tests are slow |
| |
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@kernel.org> 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@kernel.org> > > > > --- > > > > To: Brendan Higgins <brendan.higgins@linux.dev> > > To: David Gow <davidgow@google.com> > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > > Cc: Rae Moar <rmoar@google.com> > > Cc: linux-kselftest@vger.kernel.org > > Cc: kunit-dev@googlegroups.com > > Cc: linux-kernel@vger.kernel.org > > > > 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@kernel.org/ > > --- > > 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 [unhandled content-type:application/pgp-signature] | |