From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-11.4 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 391ABC4741F for ; Tue, 10 Nov 2020 10:35:27 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D031120679 for ; Tue, 10 Nov 2020 10:35:26 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="iihJcLKg" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728272AbgKJKf0 (ORCPT ); Tue, 10 Nov 2020 05:35:26 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52340 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726280AbgKJKfZ (ORCPT ); Tue, 10 Nov 2020 05:35:25 -0500 Received: from mail-ot1-x342.google.com (mail-ot1-x342.google.com [IPv6:2607:f8b0:4864:20::342]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B2D8BC0613D1 for ; Tue, 10 Nov 2020 02:35:23 -0800 (PST) Received: by mail-ot1-x342.google.com with SMTP id f16so11995849otl.11 for ; Tue, 10 Nov 2020 02:35:23 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=O9J78ygcMCgUaBelKJGuBDpmTHHQsSZqL7xxo9X1FXA=; b=iihJcLKgP4TSrgi/XOpJJLxcsC62DsZSrPSTjA7lSmML5YPgIbfpXonIcaUFoZ4xND ol7Q40wpQdgn6+48hn4iIVJ6RPMWatx/ll/y5aX3iBAd7l/30optiS9wz43TMfCYv+h2 3LULW2nv89gzaqh0QYHdlgFn2P0+LzgIU90KRsO6wMHpQ0w8DsUOkkgMf0fXnRBMJxed HtTMfxmVjzq/58yo17tXXg3HgGhrb84M40048Zb30zqZibfvPSs5A6k35uGVuJ4lFW8e 8EoBkoS3jGroVu5o/+71qHhYl/uiJ51xqICPvfWaZ+TSea4oO2tjl6O2lwl0D7vgzrKB BI5g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=O9J78ygcMCgUaBelKJGuBDpmTHHQsSZqL7xxo9X1FXA=; b=MMyj5cML+jvQ9PSaf6KX6PMIpK0paiP9+T1g+63FO1MpNuzA0fz0U26TXQNLBVRtlh savr6Xxmo2GIzoihoPkgA7hDsnOJsIrmbxDuHzsgKQZuuGEkDBuMQGYzE+FFwyGOISTc JUXxVFDZeuuteRzNDybXMBLd7TdteNWzfWSisvm+v+jGYxbhgj0v9auYrZCmI0Cxwvlb PDtHNIrsADSuuOCeyFj7awKkhWPf6Nfi0oUxgXUrTDgPtOfyyvGaILyJWE0H+ePx5cnn N6P8tqIvXJ4xKPNg8T73dLUHo16mqP7zBZCoM57MDERhKxVJfocmHy4XxAOSbKk1k4Hw cF1g== X-Gm-Message-State: AOAM533oxKkQjzUhTYSY89EBkWt0l0HLQ9PTxl1WW4Yw9+lhnNMKSkOn nX8+17TSB8zHJpKH9qCdmOHrK6YOPcBbUVRVnsQjNg== X-Google-Smtp-Source: ABdhPJwnxEs/Uy6X8cWrXNv3gk8aEzJNoK265lQPtFA5Tp98fsZsbenX9qhYvw3oT3pOA4i1EhG2MWTej8+yAivp6AM= X-Received: by 2002:a9d:f44:: with SMTP id 62mr14385069ott.17.1605004522694; Tue, 10 Nov 2020 02:35:22 -0800 (PST) MIME-Version: 1.0 References: <20201106192154.51514-1-98.arpi@gmail.com> <47a05c5a-485d-026b-c1c3-476ed1a97856@gmail.com> In-Reply-To: From: Marco Elver Date: Tue, 10 Nov 2020 11:35:11 +0100 Message-ID: Subject: Re: [PATCH v6 1/2] kunit: Support for Parameterized Testing To: David Gow Cc: Arpitha Raghunandan <98.arpi@gmail.com>, Brendan Higgins , Shuah Khan , Iurii Zaikin , "Theodore Ts'o" , Andreas Dilger , "open list:KERNEL SELFTEST FRAMEWORK" , KUnit Development , Linux Kernel Mailing List , linux-kernel-mentees@lists.linuxfoundation.org, linux-ext4@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 10 Nov 2020 at 08:21, David Gow wrote: [...] > > > > > > The previous attempt [1] at something similar failed because it seems > > > we'd need to teach kunit-tool new tricks [2], too. > > > [1] https://lkml.kernel.org/r/20201105195503.GA2399621@elver.google.c= om > > > [2] https://lkml.kernel.org/r/20201106123433.GA3563235@elver.google.c= om > > > > > > So if we go with a different format, we might need a patch before thi= s > > > one to make kunit-tool compatible with that type of diagnostic. > > > > > > Currently I think we have the following proposals for a format: > > > > > > 1. The current "# [test_case->name]: param-[index] [ok|not ok]" -- > > > this works well, because no changes to kunit-tool are required, and i= t > > > also picks up the diagnostic context for the case and displays that o= n > > > test failure. > > > > > > 2. Your proposed "# [ok|not ok] - [test_case->name]:param-[index]". > > > As-is, this needs a patch for kunit-tool as well. I just checked, and > > > if we change it to "# [ok|not ok] - [test_case->name]: param-[index]" > > > (note the space after ':') it works without changing kunit-tool. ;-) > > > > > > 3. Something like "# [ok|not ok] param-[index] - [test_case->name]", > > > which I had played with earlier but kunit-tool is definitely not yet > > > happy with. > > > > > > So my current preference is (2) with the extra space (no change to > > > kunit-tool required). WDYT? > > > > > Hmm=E2=80=A6 that failure in kunit_tool is definitely a bug: we shouldn't= care > what comes after the comment character except if it's an explicit > subtest declaration or a crash. I'll try to put a patch together to > fix it, but I'd rather not delay this just for that. > > In any thought about this a bit more, It turns out that the proposed > KTAP spec[1] discourages the use of ':', except as part of a subtest > declaration, or perhaps an as-yet-unspecified fully-qualified test > name. The latter is what I was going for, but if it's actively > breaking kunit_tool, we might want to hold off on it. > > If we were to try to treat these as subtests in accordance with that > spec, the way we'd want to use one of these options: > A) "[ok|not ok] [index] - param-[index]" -- This doesn't mention the > test case name, but otherwise treats things exactly the same way we > treat existing subtests. > > B) "[ok|not ok] [index] - [test_case->name]" -- This doesn't name the > "subtest", just gives repeated results with the same name. > > C) "[ok|not ok] [index] - [test_case->name][separator]param-[index]" > -- This is equivalent to my suggestion with a separator of ":", or 2 > above with a separator of ": ". The in-progress spec doesn't yet > specify how these fully-qualified names would work, other than that > they'd use a colon somewhere, and if we comment it out, ": " is > required. > > > > > Which format do we finally go with? > > > > I'm actually going to make another wild suggestion for this, which is > a combination of (1) and (A): > "# [test_case->name]: [ok|not ok] [index] - param-[index]" > > This gives us a KTAP-compliant result line, except prepended with "# > [test_case->name]: ", which makes it formally a diagnostic line, > rather than an actual subtest. Putting the test name at the start > matches what kunit_tool is expecting at the moment. If we then want to > turn it into a proper subtest, we can just get rid of that prefix (and > add the appropriate counts elsewhere). > > Does that sound good? Sounds reasonable to me! The repetition of [index] seems unnecessary for now, but I think if we at some point have a way to get a string representation of a param, we can substitute param-[index] with a string that represents the param. Note that once we want to make it a real subtest, we'd need to run the generator twice, once to get the number of params and then to run the tests. If we require that param generators are deterministic in the number of params generated, this is not a problem. Thanks, -- Marco