mbox series

[v3,0/5] Fix nits in the kunit

Message ID 1575396508-21480-1-git-send-email-sj38.park@gmail.com (mailing list archive)
Headers show
Series Fix nits in the kunit | expand

Message

SeongJae Park Dec. 3, 2019, 6:08 p.m. UTC 
This patchset contains trivial fixes for the kunit documentations and the
wrapper python scripts.

Changes from v2 (https://lore.kernel.org/linux-kselftest/1575361141-6806-1-git-send-email-sj38.park@gmail.com/T/#t):
 - Make 'build_dir' if not exists (missed from v3 by mistake)

SeongJae Park (5):
  docs/kunit/start: Use in-tree 'kunit_defconfig'
  kunit: Remove duplicated defconfig creation
  kunit: Create default config in '--build_dir'
  kunit: Place 'test.log' under the 'build_dir'
  kunit: Rename 'kunitconfig' to '.kunitconfig'

 Documentation/dev-tools/kunit/start.rst | 13 +++++--------
 tools/testing/kunit/kunit.py            | 16 ++++++++++------
 tools/testing/kunit/kunit_kernel.py     |  8 ++++----
 3 files changed, 19 insertions(+), 18 deletions(-)

Comments

Brendan Higgins Dec. 3, 2019, 10:41 p.m. UTC | #1 
On Tue, Dec 3, 2019 at 10:08 AM SeongJae Park <sj38.park@gmail.com> wrote:
>
> This patchset contains trivial fixes for the kunit documentations and the
> wrapper python scripts.
>
> Changes from v2 (https://lore.kernel.org/linux-kselftest/1575361141-6806-1-git-send-email-sj38.park@gmail.com/T/#t):
>  - Make 'build_dir' if not exists (missed from v3 by mistake)
>
> SeongJae Park (5):
>   docs/kunit/start: Use in-tree 'kunit_defconfig'
>   kunit: Remove duplicated defconfig creation
>   kunit: Create default config in '--build_dir'
>   kunit: Place 'test.log' under the 'build_dir'
>   kunit: Rename 'kunitconfig' to '.kunitconfig'
>
>  Documentation/dev-tools/kunit/start.rst | 13 +++++--------
>  tools/testing/kunit/kunit.py            | 16 ++++++++++------
>  tools/testing/kunit/kunit_kernel.py     |  8 ++++----
>  3 files changed, 19 insertions(+), 18 deletions(-)

Tested-by: Brendan Higgins <brendanhiggins@google.com>

Thanks!
Brendan Higgins Dec. 4, 2019, 7:21 p.m. UTC | #2 
On Tue, Dec 03, 2019 at 02:41:26PM -0800, Brendan Higgins wrote:
> On Tue, Dec 3, 2019 at 10:08 AM SeongJae Park <sj38.park@gmail.com> wrote:
> >
> > This patchset contains trivial fixes for the kunit documentations and the
> > wrapper python scripts.
> >
> > Changes from v2 (https://lore.kernel.org/linux-kselftest/1575361141-6806-1-git-send-email-sj38.park@gmail.com/T/#t):
> >  - Make 'build_dir' if not exists (missed from v3 by mistake)
> >
> > SeongJae Park (5):
> >   docs/kunit/start: Use in-tree 'kunit_defconfig'
> >   kunit: Remove duplicated defconfig creation
> >   kunit: Create default config in '--build_dir'
> >   kunit: Place 'test.log' under the 'build_dir'
> >   kunit: Rename 'kunitconfig' to '.kunitconfig'
> >
> >  Documentation/dev-tools/kunit/start.rst | 13 +++++--------
> >  tools/testing/kunit/kunit.py            | 16 ++++++++++------
> >  tools/testing/kunit/kunit_kernel.py     |  8 ++++----
> >  3 files changed, 19 insertions(+), 18 deletions(-)
> 
> Tested-by: Brendan Higgins <brendanhiggins@google.com>

I just realized that I forgot to test for something...

The following command fails:

./tools/testing/kunit/kunit.py run --timeout=60 --jobs=12 --defconfig

[11:17:13] Building KUnit Kernel ...
[11:17:16] Starting KUnit Kernel ...
Traceback (most recent call last):
  File "tools/testing/kunit/kunit.py", line 142, in <module>
    main(sys.argv[1:])
  File "tools/testing/kunit/kunit.py", line 135, in main
    result = run_tests(linux, request)
  File "tools/testing/kunit/kunit.py", line 67, in run_tests
    test_result = kunit_parser.parse_run_tests(kunit_output)
  File "/usr/local/google/home/brendanhiggins/gbmc-linux/tools/testing/kunit/kunit_parser.py", line 283, in parse_run_tests
    test_result = parse_test_result(list(isolate_kunit_output(kernel_output)))
  File "/usr/local/google/home/brendanhiggins/gbmc-linux/tools/testing/kunit/kunit_parser.py", line 54, in isolate_kunit_output
    for line in kernel_output:
  File "/usr/local/google/home/brendanhiggins/gbmc-linux/tools/testing/kunit/kunit_kernel.py", line 146, in run_kernel
    with open(os.path.join(build_dir, 'test.log'), 'w') as f:
  File "/usr/lib/python3.7/posixpath.py", line 80, in join
    a = os.fspath(a)
TypeError: expected str, bytes or os.PathLike object, not NoneType

It seems as though you assume that build_dir is always populated by the flag.
SeongJae Park Dec. 4, 2019, 8:25 p.m. UTC | #3 
On Wed, Dec 4, 2019 at 8:21 PM Brendan Higgins
<brendanhiggins@google.com> wrote:
>
> On Tue, Dec 03, 2019 at 02:41:26PM -0800, Brendan Higgins wrote:
> > On Tue, Dec 3, 2019 at 10:08 AM SeongJae Park <sj38.park@gmail.com> wrote:
> > >
> > > This patchset contains trivial fixes for the kunit documentations and the
> > > wrapper python scripts.
> > >
> > > Changes from v2 (https://lore.kernel.org/linux-kselftest/1575361141-6806-1-git-send-email-sj38.park@gmail.com/T/#t):
> > >  - Make 'build_dir' if not exists (missed from v3 by mistake)
> > >
> > > SeongJae Park (5):
> > >   docs/kunit/start: Use in-tree 'kunit_defconfig'
> > >   kunit: Remove duplicated defconfig creation
> > >   kunit: Create default config in '--build_dir'
> > >   kunit: Place 'test.log' under the 'build_dir'
> > >   kunit: Rename 'kunitconfig' to '.kunitconfig'
> > >
> > >  Documentation/dev-tools/kunit/start.rst | 13 +++++--------
> > >  tools/testing/kunit/kunit.py            | 16 ++++++++++------
> > >  tools/testing/kunit/kunit_kernel.py     |  8 ++++----
> > >  3 files changed, 19 insertions(+), 18 deletions(-)
> >
> > Tested-by: Brendan Higgins <brendanhiggins@google.com>
>
> I just realized that I forgot to test for something...
>
> The following command fails:
>
> ./tools/testing/kunit/kunit.py run --timeout=60 --jobs=12 --defconfig
>
> [11:17:13] Building KUnit Kernel ...
> [11:17:16] Starting KUnit Kernel ...
> Traceback (most recent call last):
>   File "tools/testing/kunit/kunit.py", line 142, in <module>
>     main(sys.argv[1:])
>   File "tools/testing/kunit/kunit.py", line 135, in main
>     result = run_tests(linux, request)
>   File "tools/testing/kunit/kunit.py", line 67, in run_tests
>     test_result = kunit_parser.parse_run_tests(kunit_output)
>   File "/usr/local/google/home/brendanhiggins/gbmc-linux/tools/testing/kunit/kunit_parser.py", line 283, in parse_run_tests
>     test_result = parse_test_result(list(isolate_kunit_output(kernel_output)))
>   File "/usr/local/google/home/brendanhiggins/gbmc-linux/tools/testing/kunit/kunit_parser.py", line 54, in isolate_kunit_output
>     for line in kernel_output:
>   File "/usr/local/google/home/brendanhiggins/gbmc-linux/tools/testing/kunit/kunit_kernel.py", line 146, in run_kernel
>     with open(os.path.join(build_dir, 'test.log'), 'w') as f:
>   File "/usr/lib/python3.7/posixpath.py", line 80, in join
>     a = os.fspath(a)
> TypeError: expected str, bytes or os.PathLike object, not NoneType
>
> It seems as though you assume that build_dir is always populated by the flag.

Sorry for not checking the case.  The 4th patch, "kunit: Place 'test.log' under
the 'build_dir'" made the bug.  I fixed the 4th patch and tested with below
commands:

    ./tools/testing/kunit/kunit.py run --timeout=60 --jobs=12
--defconfig --build_dir .kunit
    ./tools/testing/kunit/kunit.py run --timeout=60 --jobs=12 --defconfig

Just sent the 4th version patchset including the fix:
    http://lkml.kernel.org/r/1575490683-13015-1-git-send-email-sj38.park@gmail.com

I will consider adding some tests that can check such cases in the
'kunit_tools_test.py' later.


Thanks,
SeongJae Park
SeongJae Park Dec. 4, 2019, 8:28 p.m. UTC | #4 
On Wed, Dec 4, 2019 at 9:25 PM SeongJae Park <sj38.park@gmail.com> wrote:
>
> On Wed, Dec 4, 2019 at 8:21 PM Brendan Higgins
> <brendanhiggins@google.com> wrote:
> >
> > On Tue, Dec 03, 2019 at 02:41:26PM -0800, Brendan Higgins wrote:
> > > On Tue, Dec 3, 2019 at 10:08 AM SeongJae Park <sj38.park@gmail.com> wrote:
> > > >
> > > > This patchset contains trivial fixes for the kunit documentations and the
> > > > wrapper python scripts.
> > > >
> > > > Changes from v2 (https://lore.kernel.org/linux-kselftest/1575361141-6806-1-git-send-email-sj38.park@gmail.com/T/#t):
> > > >  - Make 'build_dir' if not exists (missed from v3 by mistake)
> > > >
> > > > SeongJae Park (5):
> > > >   docs/kunit/start: Use in-tree 'kunit_defconfig'
> > > >   kunit: Remove duplicated defconfig creation
> > > >   kunit: Create default config in '--build_dir'
> > > >   kunit: Place 'test.log' under the 'build_dir'
> > > >   kunit: Rename 'kunitconfig' to '.kunitconfig'
> > > >
> > > >  Documentation/dev-tools/kunit/start.rst | 13 +++++--------
> > > >  tools/testing/kunit/kunit.py            | 16 ++++++++++------
> > > >  tools/testing/kunit/kunit_kernel.py     |  8 ++++----
> > > >  3 files changed, 19 insertions(+), 18 deletions(-)
> > >
> > > Tested-by: Brendan Higgins <brendanhiggins@google.com>
> >
> > I just realized that I forgot to test for something...
> >
> > The following command fails:
> >
> > ./tools/testing/kunit/kunit.py run --timeout=60 --jobs=12 --defconfig
> >
> > [11:17:13] Building KUnit Kernel ...
> > [11:17:16] Starting KUnit Kernel ...
> > Traceback (most recent call last):
> >   File "tools/testing/kunit/kunit.py", line 142, in <module>
> >     main(sys.argv[1:])
> >   File "tools/testing/kunit/kunit.py", line 135, in main
> >     result = run_tests(linux, request)
> >   File "tools/testing/kunit/kunit.py", line 67, in run_tests
> >     test_result = kunit_parser.parse_run_tests(kunit_output)
> >   File "/usr/local/google/home/brendanhiggins/gbmc-linux/tools/testing/kunit/kunit_parser.py", line 283, in parse_run_tests
> >     test_result = parse_test_result(list(isolate_kunit_output(kernel_output)))
> >   File "/usr/local/google/home/brendanhiggins/gbmc-linux/tools/testing/kunit/kunit_parser.py", line 54, in isolate_kunit_output
> >     for line in kernel_output:
> >   File "/usr/local/google/home/brendanhiggins/gbmc-linux/tools/testing/kunit/kunit_kernel.py", line 146, in run_kernel
> >     with open(os.path.join(build_dir, 'test.log'), 'w') as f:
> >   File "/usr/lib/python3.7/posixpath.py", line 80, in join
> >     a = os.fspath(a)
> > TypeError: expected str, bytes or os.PathLike object, not NoneType
> >
> > It seems as though you assume that build_dir is always populated by the flag.
>
> Sorry for not checking the case.  The 4th patch, "kunit: Place 'test.log' under
> the 'build_dir'" made the bug.  I fixed the 4th patch and tested with below
> commands:
>
>     ./tools/testing/kunit/kunit.py run --timeout=60 --jobs=12
> --defconfig --build_dir .kunit
>     ./tools/testing/kunit/kunit.py run --timeout=60 --jobs=12 --defconfig
>
> Just sent the 4th version patchset including the fix:
>     http://lkml.kernel.org/r/1575490683-13015-1-git-send-email-sj38.park@gmail.com

Also, removed the 'Reviewed-by' from the 4th patch and didn't add 'Tested-by'
to the patchset.


Thanks,
SeongJae Park

>
> I will consider adding some tests that can check such cases in the
> 'kunit_tools_test.py' later.
>
>
> Thanks,
> SeongJae Park
SeongJae Park Dec. 5, 2019, 9:44 a.m. UTC | #5 
On 04.12.19 21:28, SeongJae Park wrote:
> On Wed, Dec 4, 2019 at 9:25 PM SeongJae Park <sj38.park@gmail.com> wrote:
>> On Wed, Dec 4, 2019 at 8:21 PM Brendan Higgins
>> <brendanhiggins@google.com> wrote:
>>> On Tue, Dec 03, 2019 at 02:41:26PM -0800, Brendan Higgins wrote:
>>>> On Tue, Dec 3, 2019 at 10:08 AM SeongJae Park <sj38.park@gmail.com> wrote:
>>>>> This patchset contains trivial fixes for the kunit documentations and the
>>>>> wrapper python scripts.
>>>>>
>>>>> Changes from v2 (https://lore.kernel.org/linux-kselftest/1575361141-6806-1-git-send-email-sj38.park@gmail.com/T/#t):
>>>>>  - Make 'build_dir' if not exists (missed from v3 by mistake)
>>>>>
>>>>> SeongJae Park (5):
>>>>>   docs/kunit/start: Use in-tree 'kunit_defconfig'
>>>>>   kunit: Remove duplicated defconfig creation
>>>>>   kunit: Create default config in '--build_dir'
>>>>>   kunit: Place 'test.log' under the 'build_dir'
>>>>>   kunit: Rename 'kunitconfig' to '.kunitconfig'
>>>>>
>>>>>  Documentation/dev-tools/kunit/start.rst | 13 +++++--------
>>>>>  tools/testing/kunit/kunit.py            | 16 ++++++++++------
>>>>>  tools/testing/kunit/kunit_kernel.py     |  8 ++++----
>>>>>  3 files changed, 19 insertions(+), 18 deletions(-)
>>>> Tested-by: Brendan Higgins <brendanhiggins@google.com>
>>> I just realized that I forgot to test for something...
>>>
>>> The following command fails:
>>>
>>> ./tools/testing/kunit/kunit.py run --timeout=60 --jobs=12 --defconfig
>>>
>>> [11:17:13] Building KUnit Kernel ...
>>> [11:17:16] Starting KUnit Kernel ...
>>> Traceback (most recent call last):
>>>   File "tools/testing/kunit/kunit.py", line 142, in <module>
>>>     main(sys.argv[1:])
>>>   File "tools/testing/kunit/kunit.py", line 135, in main
>>>     result = run_tests(linux, request)
>>>   File "tools/testing/kunit/kunit.py", line 67, in run_tests
>>>     test_result = kunit_parser.parse_run_tests(kunit_output)
>>>   File "/usr/local/google/home/brendanhiggins/gbmc-linux/tools/testing/kunit/kunit_parser.py", line 283, in parse_run_tests
>>>     test_result = parse_test_result(list(isolate_kunit_output(kernel_output)))
>>>   File "/usr/local/google/home/brendanhiggins/gbmc-linux/tools/testing/kunit/kunit_parser.py", line 54, in isolate_kunit_output
>>>     for line in kernel_output:
>>>   File "/usr/local/google/home/brendanhiggins/gbmc-linux/tools/testing/kunit/kunit_kernel.py", line 146, in run_kernel
>>>     with open(os.path.join(build_dir, 'test.log'), 'w') as f:
>>>   File "/usr/lib/python3.7/posixpath.py", line 80, in join
>>>     a = os.fspath(a)
>>> TypeError: expected str, bytes or os.PathLike object, not NoneType
>>>
>>> It seems as though you assume that build_dir is always populated by the flag.
>> Sorry for not checking the case.  The 4th patch, "kunit: Place 'test.log' under
>> the 'build_dir'" made the bug.  I fixed the 4th patch and tested with below
>> commands:
>>
>>     ./tools/testing/kunit/kunit.py run --timeout=60 --jobs=12
>> --defconfig --build_dir .kunit
>>     ./tools/testing/kunit/kunit.py run --timeout=60 --jobs=12 --defconfig
>>
>> Just sent the 4th version patchset including the fix:
>>     http://lkml.kernel.org/r/1575490683-13015-1-git-send-email-sj38.park@gmail.com
> Also, removed the 'Reviewed-by' from the 4th patch and didn't add 'Tested-by'
> to the patchset.
>
>
> Thanks,
> SeongJae Park
>
>> I will consider adding some tests that can check such cases in the
>> 'kunit_tools_test.py' later.

I just sent 5th version which includes this change and a fix:
https://lore.kernel.org/linux-kselftest/20191205093440.21824-1-sjpark@amazon.com


Thanks,
SeongJae Park

>>
>>
>> Thanks,
>> SeongJae Park