Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37;
MIME-Version: 1.0
References: <20231207213410.417564-1-rmoar@google.com> <20231207213410.417564-2-rmoar@google.com>
In-Reply-To: <20231207213410.417564-2-rmoar@google.com>
From:   David Gow <davidgow@google.com>
Date:   Fri, 8 Dec 2023 11:13:03 +0800
Message-ID: <CABVgOSkqH0Gsw-w5Dkm2dRfFTOd5_zhBawn0Lwgf+rkckRWpfQ@mail.gmail.com>
Subject: Re: [PATCH v2 2/2] kunit: tool: add test for parsing attributes
To:     Rae Moar <rmoar@google.com>
Cc:     shuah@kernel.org, dlatypov@google.com, brendan.higgins@linux.dev,
        linux-kselftest@vger.kernel.org, kunit-dev@googlegroups.com,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, 8 Dec 2023 at 05:34, Rae Moar <rmoar@google.com> wrote:
>
> Add test for parsing attributes to kunit_tool_test.py. Test checks
> attributes are parsed and saved in the test logs.
>
> This test also checks that the attributes have not interfered with the
> parsing of other test information, specifically the suite header as
> the test plan was being incorrectely parsed.
>
> Signed-off-by: Rae Moar <rmoar@google.com>
> ---

Thanks -- it's good to have this tested. I'm looking forward to our
actually parsing attributes out and using them for things in the
future, too.

Reviewed-by: David Gow <davidgow@google.com>

Cheers,
-- David

>  tools/testing/kunit/kunit_tool_test.py           | 16 ++++++++++++++++
>  .../kunit/test_data/test_parse_attributes.log    |  9 +++++++++
>  2 files changed, 25 insertions(+)
>  create mode 100644 tools/testing/kunit/test_data/test_parse_attributes.log
>
> diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
> index b28c1510be2e..2beb7327e53f 100755
> --- a/tools/testing/kunit/kunit_tool_test.py
> +++ b/tools/testing/kunit/kunit_tool_test.py
> @@ -331,6 +331,22 @@ class KUnitParserTest(unittest.TestCase):
>                         kunit_parser.parse_run_tests(file.readlines())
>                 self.print_mock.assert_any_call(StrContains('suite (1 subtest)'))
>
> +       def test_parse_attributes(self):
> +               ktap_log = test_data_path('test_parse_attributes.log')
> +               with open(ktap_log) as file:
> +                       result = kunit_parser.parse_run_tests(file.readlines())
> +
> +               # Test should pass with no errors
> +               self.assertEqual(result.counts, kunit_parser.TestCounts(passed=1, errors=0))
> +               self.assertEqual(kunit_parser.TestStatus.SUCCESS, result.status)
> +
> +               # Ensure suite header is parsed correctly
> +               self.print_mock.assert_any_call(StrContains('suite (1 subtest)'))
> +
> +               # Ensure attributes in correct test log
> +               self.assertContains('# module: example', result.subtests[0].log)
> +               self.assertContains('# test.speed: slow', result.subtests[0].subtests[0].log)
> +
>         def test_show_test_output_on_failure(self):
>                 output = """
>                 KTAP version 1
> diff --git a/tools/testing/kunit/test_data/test_parse_attributes.log b/tools/testing/kunit/test_data/test_parse_attributes.log
> new file mode 100644
> index 000000000000..1a13c371fe9d
> --- /dev/null
> +++ b/tools/testing/kunit/test_data/test_parse_attributes.log
> @@ -0,0 +1,9 @@
> +KTAP version 1
> +1..1
> +  KTAP version 1
> +  # Subtest: suite
> +  # module: example
> +  1..1
> +  # test.speed: slow
> +  ok 1 test
> +ok 1 suite
> \ No newline at end of file

Why doesn't this have a newline at the end of the file?

I'm actually okay with it here (it's good to test the case where there
isn't one _somewhere_), but in general, I think we'd want to prefer
KTAP documents end with the trailing newline, like most text file.