From: SeongJae Park <[email protected]>
The kunit 'Getting Started' document first shows the wrapper running
command. However, a new user who simply following the command might
encounter a failure like below:
$ ./tools/testing/kunit/kunit.py run
Traceback (most recent call last):
File "./tools/testing/kunit/kunit.py", line 140, in <module>
main(sys.argv[1:])
File "./tools/testing/kunit/kunit.py", line 126, in main
linux = kunit_kernel.LinuxSourceTree()
File "/home/sjpark/linux/tools/testing/kunit/kunit_kernel.py", line 85, in __init__
self._kconfig.read_from_file(KUNITCONFIG_PATH)
File "/home/sjpark/linux/tools/testing/kunit/kunit_config.py", line 65, in read_from_file
with open(path, 'r') as f:
FileNotFoundError: [Errno 2] No such file or directory: 'kunitconfig'
Though the reason of the failure ('kunitconfig') is explained in its
next section, it would be better to reduce any failure that user might
encounter. This commit removes the example command for the reason.
Signed-off-by: SeongJae Park <[email protected]>
---
Documentation/dev-tools/kunit/start.rst | 6 ------
1 file changed, 6 deletions(-)
diff --git a/Documentation/dev-tools/kunit/start.rst b/Documentation/dev-tools/kunit/start.rst
index 78a0aed..e25978d 100644
--- a/Documentation/dev-tools/kunit/start.rst
+++ b/Documentation/dev-tools/kunit/start.rst
@@ -15,12 +15,6 @@ Included with KUnit is a simple Python wrapper that helps format the output to
easily use and read KUnit output. It handles building and running the kernel, as
well as formatting the output.
-The wrapper can be run with:
-
-.. code-block:: bash
-
- ./tools/testing/kunit/kunit.py run
-
Creating a kunitconfig
======================
The Python script is a thin wrapper around Kbuild as such, it needs to be
--
2.7.4
+David Gow - David has lots of good opinions on our documentation.
On Sun, Dec 1, 2019 at 3:25 PM SeongJae Park <[email protected]> wrote:
>
> From: SeongJae Park <[email protected]>
>
> The kunit 'Getting Started' document first shows the wrapper running
> command. However, a new user who simply following the command might
> encounter a failure like below:
>
> $ ./tools/testing/kunit/kunit.py run
> Traceback (most recent call last):
> File "./tools/testing/kunit/kunit.py", line 140, in <module>
> main(sys.argv[1:])
> File "./tools/testing/kunit/kunit.py", line 126, in main
> linux = kunit_kernel.LinuxSourceTree()
> File "/home/sjpark/linux/tools/testing/kunit/kunit_kernel.py", line 85, in __init__
> self._kconfig.read_from_file(KUNITCONFIG_PATH)
> File "/home/sjpark/linux/tools/testing/kunit/kunit_config.py", line 65, in read_from_file
> with open(path, 'r') as f:
> FileNotFoundError: [Errno 2] No such file or directory: 'kunitconfig'
>
> Though the reason of the failure ('kunitconfig') is explained in its
> next section, it would be better to reduce any failure that user might
> encounter. This commit removes the example command for the reason.
Seems reasonable.
> Signed-off-by: SeongJae Park <[email protected]>
> ---
> Documentation/dev-tools/kunit/start.rst | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/Documentation/dev-tools/kunit/start.rst b/Documentation/dev-tools/kunit/start.rst
> index 78a0aed..e25978d 100644
> --- a/Documentation/dev-tools/kunit/start.rst
> +++ b/Documentation/dev-tools/kunit/start.rst
> @@ -15,12 +15,6 @@ Included with KUnit is a simple Python wrapper that helps format the output to
> easily use and read KUnit output. It handles building and running the kernel, as
> well as formatting the output.
>
> -The wrapper can be run with:
> -
> -.. code-block:: bash
> -
> - ./tools/testing/kunit/kunit.py run
> -
> Creating a kunitconfig
> ======================
I think maybe we should demote this section so that this is a
subsection under KUnit Wrapper. Might also want to add a tie-in
explaining why we are talking about kunitconfig here? Right now this
kind of reads as a non sequitur.
Note: we have tried to address this potential issue for new users in
this patch under review:
https://patchwork.kernel.org/patch/11252953/
I don't feel strongly whether we do it your way or my way. What do
other people think?
> The Python script is a thin wrapper around Kbuild as such, it needs to be
> --
> 2.7.4
>
On Mon, Dec 2, 2019 at 6:25 PM Brendan Higgins
<[email protected]> wrote:
>
> +David Gow - David has lots of good opinions on our documentation.
>
> On Sun, Dec 1, 2019 at 3:25 PM SeongJae Park <[email protected]> wrote:
> >
> > From: SeongJae Park <[email protected]>
> >
> > The kunit 'Getting Started' document first shows the wrapper running
> > command. However, a new user who simply following the command might
> > encounter a failure like below:
> >
> > $ ./tools/testing/kunit/kunit.py run
> > Traceback (most recent call last):
> > File "./tools/testing/kunit/kunit.py", line 140, in <module>
> > main(sys.argv[1:])
> > File "./tools/testing/kunit/kunit.py", line 126, in main
> > linux = kunit_kernel.LinuxSourceTree()
> > File "/home/sjpark/linux/tools/testing/kunit/kunit_kernel.py", line 85, in __init__
> > self._kconfig.read_from_file(KUNITCONFIG_PATH)
> > File "/home/sjpark/linux/tools/testing/kunit/kunit_config.py", line 65, in read_from_file
> > with open(path, 'r') as f:
> > FileNotFoundError: [Errno 2] No such file or directory: 'kunitconfig'
> >
> > Though the reason of the failure ('kunitconfig') is explained in its
> > next section, it would be better to reduce any failure that user might
> > encounter. This commit removes the example command for the reason.
>
> Seems reasonable.
>
> > Signed-off-by: SeongJae Park <[email protected]>
> > ---
> > Documentation/dev-tools/kunit/start.rst | 6 ------
> > 1 file changed, 6 deletions(-)
> >
> > diff --git a/Documentation/dev-tools/kunit/start.rst b/Documentation/dev-tools/kunit/start.rst
> > index 78a0aed..e25978d 100644
> > --- a/Documentation/dev-tools/kunit/start.rst
> > +++ b/Documentation/dev-tools/kunit/start.rst
> > @@ -15,12 +15,6 @@ Included with KUnit is a simple Python wrapper that helps format the output to
> > easily use and read KUnit output. It handles building and running the kernel, as
> > well as formatting the output.
> >
> > -The wrapper can be run with:
> > -
> > -.. code-block:: bash
> > -
> > - ./tools/testing/kunit/kunit.py run
> > -
> > Creating a kunitconfig
> > ======================
>
> I think maybe we should demote this section so that this is a
> subsection under KUnit Wrapper. Might also want to add a tie-in
> explaining why we are talking about kunitconfig here? Right now this
> kind of reads as a non sequitur.
>
> Note: we have tried to address this potential issue for new users in
> this patch under review:
>
> https://patchwork.kernel.org/patch/11252953/
>
> I don't feel strongly whether we do it your way or my way. What do
> other people think?
I totally agree your way :)
Thanks,
SeongJae Park
>
> > The Python script is a thin wrapper around Kbuild as such, it needs to be
> > --
> > 2.7.4
> >
On Mon, Dec 2, 2019 at 9:25 AM Brendan Higgins
<[email protected]> wrote:
>
> +David Gow - David has lots of good opinions on our documentation.
>
> On Sun, Dec 1, 2019 at 3:25 PM SeongJae Park <[email protected]> wrote:
> >
> > From: SeongJae Park <[email protected]>
> >
> > The kunit 'Getting Started' document first shows the wrapper running
> > command. However, a new user who simply following the command might
> > encounter a failure like below:
> >
> > $ ./tools/testing/kunit/kunit.py run
> > Traceback (most recent call last):
> > File "./tools/testing/kunit/kunit.py", line 140, in <module>
> > main(sys.argv[1:])
> > File "./tools/testing/kunit/kunit.py", line 126, in main
> > linux = kunit_kernel.LinuxSourceTree()
> > File "/home/sjpark/linux/tools/testing/kunit/kunit_kernel.py", line 85, in __init__
> > self._kconfig.read_from_file(KUNITCONFIG_PATH)
> > File "/home/sjpark/linux/tools/testing/kunit/kunit_config.py", line 65, in read_from_file
> > with open(path, 'r') as f:
> > FileNotFoundError: [Errno 2] No such file or directory: 'kunitconfig'
> >
> > Though the reason of the failure ('kunitconfig') is explained in its
> > next section, it would be better to reduce any failure that user might
> > encounter. This commit removes the example command for the reason.
>
> Seems reasonable.
>
I definitely agree that having a non-working command here is doing
more harm than good. Whether we just get rid of it, or change it to
use the --defconfig option is a matter of taste. (Personally, I think
there's some value in having a one-line "run the tests" command at the
top of the Getting Started page, but it definitely needs to be one
that works.)
So, overall, I think this is definitely an improvement, but that we do
need to choose whether to take this approach (deleting this command)
or the --defconfig approach as in:
https://lore.kernel.org/linux-kselftest/[email protected]/
> > Signed-off-by: SeongJae Park <[email protected]>
> > ---
> > Documentation/dev-tools/kunit/start.rst | 6 ------
> > 1 file changed, 6 deletions(-)
> >
> > diff --git a/Documentation/dev-tools/kunit/start.rst b/Documentation/dev-tools/kunit/start.rst
> > index 78a0aed..e25978d 100644
> > --- a/Documentation/dev-tools/kunit/start.rst
> > +++ b/Documentation/dev-tools/kunit/start.rst
> > @@ -15,12 +15,6 @@ Included with KUnit is a simple Python wrapper that helps format the output to
> > easily use and read KUnit output. It handles building and running the kernel, as
> > well as formatting the output.
> >
> > -The wrapper can be run with:
> > -
> > -.. code-block:: bash
> > -
> > - ./tools/testing/kunit/kunit.py run
> > -
> > Creating a kunitconfig
> > ======================
>
> I think maybe we should demote this section so that this is a
> subsection under KUnit Wrapper. Might also want to add a tie-in
> explaining why we are talking about kunitconfig here? Right now this
> kind of reads as a non sequitur.
I generally think we want to keep the "Getting Started" guide focused
on the goal (running/writing tests), rather than too much detail on
the implementation (the wrapper itself).
How about renaming what's currently the "KUnit Wrapper" section to
"Running tests" or similar, and moving the kunitconfig part under
that?
The "Creating a kunitconfig" part could equally be "configuring which
tests to run" or something, which may speak more to motivating
As for some sort of tie-in, perhaps rewording the opening sentence to
say "The easiest way to run tests is to use the kunit_tool script",
and link to the page kunit_tool page in the patch below?
>
> Note: we have tried to address this potential issue for new users in
> this patch under review:
>
> https://patchwork.kernel.org/patch/11252953/
>
> I don't feel strongly whether we do it your way or my way. What do
> other people think?
As above, my slight preference is for adding the --defconfig option
over removing the section entirely.
> > The Python script is a thin wrapper around Kbuild as such, it needs to be
> > --
> > 2.7.4
> >
On Mon, Dec 2, 2019 at 9:25 AM Brendan Higgins
<[email protected]> wrote:
>
> +David Gow - David has lots of good opinions on our documentation.
>
> On Sun, Dec 1, 2019 at 3:25 PM SeongJae Park <[email protected]> wrote:
> >
> > From: SeongJae Park <[email protected]>
> >
> > The kunit 'Getting Started' document first shows the wrapper running
> > command. However, a new user who simply following the command might
> > encounter a failure like below:
> >
> > $ ./tools/testing/kunit/kunit.py run
> > Traceback (most recent call last):
> > File "./tools/testing/kunit/kunit.py", line 140, in <module>
> > main(sys.argv[1:])
> > File "./tools/testing/kunit/kunit.py", line 126, in main
> > linux = kunit_kernel.LinuxSourceTree()
> > File "/home/sjpark/linux/tools/testing/kunit/kunit_kernel.py", line 85, in __init__
> > self._kconfig.read_from_file(KUNITCONFIG_PATH)
> > File "/home/sjpark/linux/tools/testing/kunit/kunit_config.py", line 65, in read_from_file
> > with open(path, 'r') as f:
> > FileNotFoundError: [Errno 2] No such file or directory: 'kunitconfig'
> >
> > Though the reason of the failure ('kunitconfig') is explained in its
> > next section, it would be better to reduce any failure that user might
> > encounter. This commit removes the example command for the reason.
>
> Seems reasonable.
>
> > Signed-off-by: SeongJae Park <[email protected]>
> > ---
> > Documentation/dev-tools/kunit/start.rst | 6 ------
> > 1 file changed, 6 deletions(-)
> >
> > diff --git a/Documentation/dev-tools/kunit/start.rst b/Documentation/dev-tools/kunit/start.rst
> > index 78a0aed..e25978d 100644
> > --- a/Documentation/dev-tools/kunit/start.rst
> > +++ b/Documentation/dev-tools/kunit/start.rst
> > @@ -15,12 +15,6 @@ Included with KUnit is a simple Python wrapper that helps format the output to
> > easily use and read KUnit output. It handles building and running the kernel, as
> > well as formatting the output.
> >
> > -The wrapper can be run with:
> > -
> > -.. code-block:: bash
> > -
> > - ./tools/testing/kunit/kunit.py run
> > -
> > Creating a kunitconfig
> > ======================
>
> I think maybe we should demote this section so that this is a
> subsection under KUnit Wrapper. Might also want to add a tie-in
> explaining why we are talking about kunitconfig here? Right now this
> kind of reads as a non sequitur.
>
> Note: we have tried to address this potential issue for new users in
> this patch under review:
>
> https://patchwork.kernel.org/patch/11252953/
>
> I don't feel strongly whether we do it your way or my way. What do
> other people think?
>
> > The Python script is a thin wrapper around Kbuild as such, it needs to be
> > --
> > 2.7.4
> >
On Mon, Dec 2, 2019 at 10:03 PM David Gow <[email protected]> wrote:
>
> On Mon, Dec 2, 2019 at 9:25 AM Brendan Higgins
> <[email protected]> wrote:
> >
> > +David Gow - David has lots of good opinions on our documentation.
> >
> > On Sun, Dec 1, 2019 at 3:25 PM SeongJae Park <[email protected]> wrote:
> > >
> > > From: SeongJae Park <[email protected]>
> > >
> > > The kunit 'Getting Started' document first shows the wrapper running
> > > command. However, a new user who simply following the command might
> > > encounter a failure like below:
> > >
> > > $ ./tools/testing/kunit/kunit.py run
> > > Traceback (most recent call last):
> > > File "./tools/testing/kunit/kunit.py", line 140, in <module>
> > > main(sys.argv[1:])
> > > File "./tools/testing/kunit/kunit.py", line 126, in main
> > > linux = kunit_kernel.LinuxSourceTree()
> > > File "/home/sjpark/linux/tools/testing/kunit/kunit_kernel.py", line 85, in __init__
> > > self._kconfig.read_from_file(KUNITCONFIG_PATH)
> > > File "/home/sjpark/linux/tools/testing/kunit/kunit_config.py", line 65, in read_from_file
> > > with open(path, 'r') as f:
> > > FileNotFoundError: [Errno 2] No such file or directory: 'kunitconfig'
> > >
> > > Though the reason of the failure ('kunitconfig') is explained in its
> > > next section, it would be better to reduce any failure that user might
> > > encounter. This commit removes the example command for the reason.
> >
> > Seems reasonable.
> >
> I definitely agree that having a non-working command here is doing
> more harm than good. Whether we just get rid of it, or change it to
> use the --defconfig option is a matter of taste. (Personally, I think
> there's some value in having a one-line "run the tests" command at the
> top of the Getting Started page, but it definitely needs to be one
> that works.)
>
> So, overall, I think this is definitely an improvement, but that we do
> need to choose whether to take this approach (deleting this command)
> or the --defconfig approach as in:
> https://lore.kernel.org/linux-kselftest/[email protected]/
>
> > > Signed-off-by: SeongJae Park <[email protected]>
> > > ---
> > > Documentation/dev-tools/kunit/start.rst | 6 ------
> > > 1 file changed, 6 deletions(-)
> > >
> > > diff --git a/Documentation/dev-tools/kunit/start.rst b/Documentation/dev-tools/kunit/start.rst
> > > index 78a0aed..e25978d 100644
> > > --- a/Documentation/dev-tools/kunit/start.rst
> > > +++ b/Documentation/dev-tools/kunit/start.rst
> > > @@ -15,12 +15,6 @@ Included with KUnit is a simple Python wrapper that helps format the output to
> > > easily use and read KUnit output. It handles building and running the kernel, as
> > > well as formatting the output.
> > >
> > > -The wrapper can be run with:
> > > -
> > > -.. code-block:: bash
> > > -
> > > - ./tools/testing/kunit/kunit.py run
> > > -
> > > Creating a kunitconfig
> > > ======================
> >
> > I think maybe we should demote this section so that this is a
> > subsection under KUnit Wrapper. Might also want to add a tie-in
> > explaining why we are talking about kunitconfig here? Right now this
> > kind of reads as a non sequitur.
> I generally think we want to keep the "Getting Started" guide focused
> on the goal (running/writing tests), rather than too much detail on
> the implementation (the wrapper itself).
> How about renaming what's currently the "KUnit Wrapper" section to
> "Running tests" or similar, and moving the kunitconfig part under
> that?
>
> The "Creating a kunitconfig" part could equally be "configuring which
> tests to run" or something, which may speak more to motivating
>
> As for some sort of tie-in, perhaps rewording the opening sentence to
> say "The easiest way to run tests is to use the kunit_tool script",
> and link to the page kunit_tool page in the patch below?
> >
> > Note: we have tried to address this potential issue for new users in
> > this patch under review:
> >
> > https://patchwork.kernel.org/patch/11252953/
> >
> > I don't feel strongly whether we do it your way or my way. What do
> > other people think?
>
> As above, my slight preference is for adding the --defconfig option
> over removing the section entirely.
Agree, I would also prefer to do explain about '--defconfig' option.
Thanks,
SeongJae Park
>
> > > The Python script is a thin wrapper around Kbuild as such, it needs to be
> > > --
> > > 2.7.4
> > >
> On Mon, Dec 2, 2019 at 9:25 AM Brendan Higgins
> <[email protected]> wrote:
> >
> > +David Gow - David has lots of good opinions on our documentation.
> >
> > On Sun, Dec 1, 2019 at 3:25 PM SeongJae Park <[email protected]> wrote:
> > >
> > > From: SeongJae Park <[email protected]>
> > >
> > > The kunit 'Getting Started' document first shows the wrapper running
> > > command. However, a new user who simply following the command might
> > > encounter a failure like below:
> > >
> > > $ ./tools/testing/kunit/kunit.py run
> > > Traceback (most recent call last):
> > > File "./tools/testing/kunit/kunit.py", line 140, in <module>
> > > main(sys.argv[1:])
> > > File "./tools/testing/kunit/kunit.py", line 126, in main
> > > linux = kunit_kernel.LinuxSourceTree()
> > > File "/home/sjpark/linux/tools/testing/kunit/kunit_kernel.py", line 85, in __init__
> > > self._kconfig.read_from_file(KUNITCONFIG_PATH)
> > > File "/home/sjpark/linux/tools/testing/kunit/kunit_config.py", line 65, in read_from_file
> > > with open(path, 'r') as f:
> > > FileNotFoundError: [Errno 2] No such file or directory: 'kunitconfig'
> > >
> > > Though the reason of the failure ('kunitconfig') is explained in its
> > > next section, it would be better to reduce any failure that user might
> > > encounter. This commit removes the example command for the reason.
> >
> > Seems reasonable.
> >
> > > Signed-off-by: SeongJae Park <[email protected]>
> > > ---
> > > Documentation/dev-tools/kunit/start.rst | 6 ------
> > > 1 file changed, 6 deletions(-)
> > >
> > > diff --git a/Documentation/dev-tools/kunit/start.rst b/Documentation/dev-tools/kunit/start.rst
> > > index 78a0aed..e25978d 100644
> > > --- a/Documentation/dev-tools/kunit/start.rst
> > > +++ b/Documentation/dev-tools/kunit/start.rst
> > > @@ -15,12 +15,6 @@ Included with KUnit is a simple Python wrapper that helps format the output to
> > > easily use and read KUnit output. It handles building and running the kernel, as
> > > well as formatting the output.
> > >
> > > -The wrapper can be run with:
> > > -
> > > -.. code-block:: bash
> > > -
> > > - ./tools/testing/kunit/kunit.py run
> > > -
> > > Creating a kunitconfig
> > > ======================
> >
> > I think maybe we should demote this section so that this is a
> > subsection under KUnit Wrapper. Might also want to add a tie-in
> > explaining why we are talking about kunitconfig here? Right now this
> > kind of reads as a non sequitur.
> >
> > Note: we have tried to address this potential issue for new users in
> > this patch under review:
> >
> > https://patchwork.kernel.org/patch/11252953/
> >
> > I don't feel strongly whether we do it your way or my way. What do
> > other people think?
> >
> > > The Python script is a thin wrapper around Kbuild as such, it needs to be
> > > --
> > > 2.7.4
> > >
On Mon, Dec 2, 2019 at 9:59 PM SeongJae Park <[email protected]> wrote:
>
> On Mon, Dec 2, 2019 at 6:25 PM Brendan Higgins
> <[email protected]> wrote:
> >
> > +David Gow - David has lots of good opinions on our documentation.
> >
> > On Sun, Dec 1, 2019 at 3:25 PM SeongJae Park <[email protected]> wrote:
> > >
> > > From: SeongJae Park <[email protected]>
> > >
> > > The kunit 'Getting Started' document first shows the wrapper running
> > > command. However, a new user who simply following the command might
> > > encounter a failure like below:
> > >
> > > $ ./tools/testing/kunit/kunit.py run
> > > Traceback (most recent call last):
> > > File "./tools/testing/kunit/kunit.py", line 140, in <module>
> > > main(sys.argv[1:])
> > > File "./tools/testing/kunit/kunit.py", line 126, in main
> > > linux = kunit_kernel.LinuxSourceTree()
> > > File "/home/sjpark/linux/tools/testing/kunit/kunit_kernel.py", line 85, in __init__
> > > self._kconfig.read_from_file(KUNITCONFIG_PATH)
> > > File "/home/sjpark/linux/tools/testing/kunit/kunit_config.py", line 65, in read_from_file
> > > with open(path, 'r') as f:
> > > FileNotFoundError: [Errno 2] No such file or directory: 'kunitconfig'
> > >
> > > Though the reason of the failure ('kunitconfig') is explained in its
> > > next section, it would be better to reduce any failure that user might
> > > encounter. This commit removes the example command for the reason.
> >
> > Seems reasonable.
> >
> > > Signed-off-by: SeongJae Park <[email protected]>
> > > ---
> > > Documentation/dev-tools/kunit/start.rst | 6 ------
> > > 1 file changed, 6 deletions(-)
> > >
> > > diff --git a/Documentation/dev-tools/kunit/start.rst b/Documentation/dev-tools/kunit/start.rst
> > > index 78a0aed..e25978d 100644
> > > --- a/Documentation/dev-tools/kunit/start.rst
> > > +++ b/Documentation/dev-tools/kunit/start.rst
> > > @@ -15,12 +15,6 @@ Included with KUnit is a simple Python wrapper that helps format the output to
> > > easily use and read KUnit output. It handles building and running the kernel, as
> > > well as formatting the output.
> > >
> > > -The wrapper can be run with:
> > > -
> > > -.. code-block:: bash
> > > -
> > > - ./tools/testing/kunit/kunit.py run
> > > -
> > > Creating a kunitconfig
> > > ======================
> >
> > I think maybe we should demote this section so that this is a
> > subsection under KUnit Wrapper. Might also want to add a tie-in
> > explaining why we are talking about kunitconfig here? Right now this
> > kind of reads as a non sequitur.
> >
> > Note: we have tried to address this potential issue for new users in
> > this patch under review:
> >
> > https://patchwork.kernel.org/patch/11252953/
> >
> > I don't feel strongly whether we do it your way or my way. What do
> > other people think?
>
> I totally agree your way :)
For the reason, I removed this patch from the second version patchset[1].
[1] https://lore.kernel.org/linux-doc/[email protected]/
Thanks,
SeongJae Park
>
>
> Thanks,
> SeongJae Park
>
> >
> > > The Python script is a thin wrapper around Kbuild as such, it needs to be
> > > --
> > > 2.7.4
> > >