| Message ID | 20190820232046.50175-2-brendanhiggins@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kunit: introduce KUnit, the Linux kernel unit testing framework | expand |
Hi Brendan, On 8/20/19 5:20 PM, Brendan Higgins wrote: > Add core facilities for defining unit tests; this provides a common way > to define test cases, functions that execute code which is under test > and determine whether the code under test behaves as expected; this also > provides a way to group together related test cases in test suites (here > we call them test_modules). > > Just define test cases and how to execute them for now; setting > expectations on code will be defined later. > > Signed-off-by: Brendan Higgins <brendanhiggins@google.com> > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Reviewed-by: Logan Gunthorpe <logang@deltatee.com> > Reviewed-by: Luis Chamberlain <mcgrof@kernel.org> > Reviewed-by: Stephen Boyd <sboyd@kernel.org> > --- > include/kunit/test.h | 179 ++++++++++++++++++++++++++++++++++++++++ > kunit/Kconfig | 17 ++++ > kunit/Makefile | 1 + > kunit/test.c | 191 +++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 388 insertions(+) > create mode 100644 include/kunit/test.h > create mode 100644 kunit/Kconfig > create mode 100644 kunit/Makefile > create mode 100644 kunit/test.c > > diff --git a/include/kunit/test.h b/include/kunit/test.h > new file mode 100644 > index 0000000000000..e0b34acb9ee4e > --- /dev/null > +++ b/include/kunit/test.h > @@ -0,0 +1,179 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Base unit test (KUnit) API. > + * > + * Copyright (C) 2019, Google LLC. > + * Author: Brendan Higgins <brendanhiggins@google.com> > + */ > + > +#ifndef _KUNIT_TEST_H > +#define _KUNIT_TEST_H > + > +#include <linux/types.h> > + > +struct kunit; > + > +/** > + * struct kunit_case - represents an individual test case. > + * @run_case: the function representing the actual test case. > + * @name: the name of the test case. > + * > + * A test case is a function with the signature, ``void (*)(struct kunit *)`` > + * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. Each > + * test case is associated with a &struct kunit_suite and will be run after the > + * suite's init function and followed by the suite's exit function. > + * > + * A test case should be static and should only be created with the KUNIT_CASE() > + * macro; additionally, every array of test cases should be terminated with an > + * empty test case. > + * > + * Example: Can you fix these line continuations. It makes it very hard to read. Sorry for this late comment. These comments lines are longer than 80 and wrap. There are several comment lines in the file that are way too long. thanks, -- Shuah
On Fri, Aug 23, 2019 at 8:33 AM shuah <shuah@kernel.org> wrote: > > Hi Brendan, > > On 8/20/19 5:20 PM, Brendan Higgins wrote: > > Add core facilities for defining unit tests; this provides a common way > > to define test cases, functions that execute code which is under test > > and determine whether the code under test behaves as expected; this also > > provides a way to group together related test cases in test suites (here > > we call them test_modules). > > > > Just define test cases and how to execute them for now; setting > > expectations on code will be defined later. > > > > Signed-off-by: Brendan Higgins <brendanhiggins@google.com> > > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Reviewed-by: Logan Gunthorpe <logang@deltatee.com> > > Reviewed-by: Luis Chamberlain <mcgrof@kernel.org> > > Reviewed-by: Stephen Boyd <sboyd@kernel.org> > > --- > > include/kunit/test.h | 179 ++++++++++++++++++++++++++++++++++++++++ > > kunit/Kconfig | 17 ++++ > > kunit/Makefile | 1 + > > kunit/test.c | 191 +++++++++++++++++++++++++++++++++++++++++++ > > 4 files changed, 388 insertions(+) > > create mode 100644 include/kunit/test.h > > create mode 100644 kunit/Kconfig > > create mode 100644 kunit/Makefile > > create mode 100644 kunit/test.c > > > > diff --git a/include/kunit/test.h b/include/kunit/test.h > > new file mode 100644 > > index 0000000000000..e0b34acb9ee4e > > --- /dev/null > > +++ b/include/kunit/test.h > > @@ -0,0 +1,179 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +/* > > + * Base unit test (KUnit) API. > > + * > > + * Copyright (C) 2019, Google LLC. > > + * Author: Brendan Higgins <brendanhiggins@google.com> > > + */ > > + > > +#ifndef _KUNIT_TEST_H > > +#define _KUNIT_TEST_H > > + > > +#include <linux/types.h> > > + > > +struct kunit; > > + > > +/** > > + * struct kunit_case - represents an individual test case. > > + * @run_case: the function representing the actual test case. > > + * @name: the name of the test case. > > + * > > + * A test case is a function with the signature, ``void (*)(struct kunit *)`` > > + * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. Each > > + * test case is associated with a &struct kunit_suite and will be run after the > > + * suite's init function and followed by the suite's exit function. > > + * > > + * A test case should be static and should only be created with the KUNIT_CASE() > > + * macro; additionally, every array of test cases should be terminated with an > > + * empty test case. > > + * > > + * Example: > > Can you fix these line continuations. It makes it very hard to read. > Sorry for this late comment. These comments lines are longer than 80 > and wrap. None of the lines in this commit are over 80 characters in column width. Some are exactly 80 characters (like above). My guess is that you are seeing the diff added text (+ ), which when you add that to a line which is exactly 80 char in length ends up being over 80 char in email. If you apply the patch you will see that they are only 80 chars. > > There are several comment lines in the file that are way too long. Note that checkpatch also does not complain about any over 80 char lines in this file. Sorry if I am misunderstanding what you are trying to tell me. Please confirm either way. Thanks
On 8/23/19 10:48 AM, Brendan Higgins wrote: > On Fri, Aug 23, 2019 at 8:33 AM shuah <shuah@kernel.org> wrote: >> >> Hi Brendan, >> >> On 8/20/19 5:20 PM, Brendan Higgins wrote: >>> Add core facilities for defining unit tests; this provides a common way >>> to define test cases, functions that execute code which is under test >>> and determine whether the code under test behaves as expected; this also >>> provides a way to group together related test cases in test suites (here >>> we call them test_modules). >>> >>> Just define test cases and how to execute them for now; setting >>> expectations on code will be defined later. >>> >>> Signed-off-by: Brendan Higgins <brendanhiggins@google.com> >>> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >>> Reviewed-by: Logan Gunthorpe <logang@deltatee.com> >>> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org> >>> Reviewed-by: Stephen Boyd <sboyd@kernel.org> >>> --- >>> include/kunit/test.h | 179 ++++++++++++++++++++++++++++++++++++++++ >>> kunit/Kconfig | 17 ++++ >>> kunit/Makefile | 1 + >>> kunit/test.c | 191 +++++++++++++++++++++++++++++++++++++++++++ >>> 4 files changed, 388 insertions(+) >>> create mode 100644 include/kunit/test.h >>> create mode 100644 kunit/Kconfig >>> create mode 100644 kunit/Makefile >>> create mode 100644 kunit/test.c >>> >>> diff --git a/include/kunit/test.h b/include/kunit/test.h >>> new file mode 100644 >>> index 0000000000000..e0b34acb9ee4e >>> --- /dev/null >>> +++ b/include/kunit/test.h >>> @@ -0,0 +1,179 @@ >>> +/* SPDX-License-Identifier: GPL-2.0 */ >>> +/* >>> + * Base unit test (KUnit) API. >>> + * >>> + * Copyright (C) 2019, Google LLC. >>> + * Author: Brendan Higgins <brendanhiggins@google.com> >>> + */ >>> + >>> +#ifndef _KUNIT_TEST_H >>> +#define _KUNIT_TEST_H >>> + >>> +#include <linux/types.h> >>> + >>> +struct kunit; >>> + >>> +/** >>> + * struct kunit_case - represents an individual test case. >>> + * @run_case: the function representing the actual test case. >>> + * @name: the name of the test case. >>> + * >>> + * A test case is a function with the signature, ``void (*)(struct kunit *)`` >>> + * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. Each >>> + * test case is associated with a &struct kunit_suite and will be run after the >>> + * suite's init function and followed by the suite's exit function. >>> + * >>> + * A test case should be static and should only be created with the KUNIT_CASE() >>> + * macro; additionally, every array of test cases should be terminated with an >>> + * empty test case. >>> + * >>> + * Example: >> >> Can you fix these line continuations. It makes it very hard to read. >> Sorry for this late comment. These comments lines are longer than 80 >> and wrap. > > None of the lines in this commit are over 80 characters in column > width. Some are exactly 80 characters (like above). > > My guess is that you are seeing the diff added text (+ ), which when > you add that to a line which is exactly 80 char in length ends up > being over 80 char in email. If you apply the patch you will see that > they are only 80 chars. > >> >> There are several comment lines in the file that are way too long. > > Note that checkpatch also does not complain about any over 80 char > lines in this file. > > Sorry if I am misunderstanding what you are trying to tell me. Please > confirm either way. > WARNING: Avoid unnecessary line continuations #258: FILE: include/kunit/test.h:137: + */ \ total: 0 errors, 2 warnings, 388 lines checked Go ahead fix these. It appears there are few lines that either longer than 80. In general, I keep them around 75, so it is easier read. thanks, -- Shuah
On Fri, Aug 23, 2019 at 10:05 AM shuah <shuah@kernel.org> wrote: > > On 8/23/19 10:48 AM, Brendan Higgins wrote: > > On Fri, Aug 23, 2019 at 8:33 AM shuah <shuah@kernel.org> wrote: > >> > >> Hi Brendan, > >> > >> On 8/20/19 5:20 PM, Brendan Higgins wrote: > >>> Add core facilities for defining unit tests; this provides a common way > >>> to define test cases, functions that execute code which is under test > >>> and determine whether the code under test behaves as expected; this also > >>> provides a way to group together related test cases in test suites (here > >>> we call them test_modules). > >>> > >>> Just define test cases and how to execute them for now; setting > >>> expectations on code will be defined later. > >>> > >>> Signed-off-by: Brendan Higgins <brendanhiggins@google.com> > >>> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > >>> Reviewed-by: Logan Gunthorpe <logang@deltatee.com> > >>> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org> > >>> Reviewed-by: Stephen Boyd <sboyd@kernel.org> > >>> --- > >>> include/kunit/test.h | 179 ++++++++++++++++++++++++++++++++++++++++ > >>> kunit/Kconfig | 17 ++++ > >>> kunit/Makefile | 1 + > >>> kunit/test.c | 191 +++++++++++++++++++++++++++++++++++++++++++ > >>> 4 files changed, 388 insertions(+) > >>> create mode 100644 include/kunit/test.h > >>> create mode 100644 kunit/Kconfig > >>> create mode 100644 kunit/Makefile > >>> create mode 100644 kunit/test.c > >>> > >>> diff --git a/include/kunit/test.h b/include/kunit/test.h > >>> new file mode 100644 > >>> index 0000000000000..e0b34acb9ee4e > >>> --- /dev/null > >>> +++ b/include/kunit/test.h > >>> @@ -0,0 +1,179 @@ > >>> +/* SPDX-License-Identifier: GPL-2.0 */ > >>> +/* > >>> + * Base unit test (KUnit) API. > >>> + * > >>> + * Copyright (C) 2019, Google LLC. > >>> + * Author: Brendan Higgins <brendanhiggins@google.com> > >>> + */ > >>> + > >>> +#ifndef _KUNIT_TEST_H > >>> +#define _KUNIT_TEST_H > >>> + > >>> +#include <linux/types.h> > >>> + > >>> +struct kunit; > >>> + > >>> +/** > >>> + * struct kunit_case - represents an individual test case. > >>> + * @run_case: the function representing the actual test case. > >>> + * @name: the name of the test case. > >>> + * > >>> + * A test case is a function with the signature, ``void (*)(struct kunit *)`` > >>> + * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. Each > >>> + * test case is associated with a &struct kunit_suite and will be run after the > >>> + * suite's init function and followed by the suite's exit function. > >>> + * > >>> + * A test case should be static and should only be created with the KUNIT_CASE() > >>> + * macro; additionally, every array of test cases should be terminated with an > >>> + * empty test case. > >>> + * > >>> + * Example: > >> > >> Can you fix these line continuations. It makes it very hard to read. > >> Sorry for this late comment. These comments lines are longer than 80 > >> and wrap. > > > > None of the lines in this commit are over 80 characters in column > > width. Some are exactly 80 characters (like above). > > > > My guess is that you are seeing the diff added text (+ ), which when > > you add that to a line which is exactly 80 char in length ends up > > being over 80 char in email. If you apply the patch you will see that > > they are only 80 chars. > > > >> > >> There are several comment lines in the file that are way too long. > > > > Note that checkpatch also does not complain about any over 80 char > > lines in this file. > > > > Sorry if I am misunderstanding what you are trying to tell me. Please > > confirm either way. > > > > WARNING: Avoid unnecessary line continuations > #258: FILE: include/kunit/test.h:137: > + */ \ > > total: 0 errors, 2 warnings, 388 lines checked Ah, okay so you don't like the warning about the line continuation. That's not because it is over 80 char, but because there is a line continuation after a comment. I don't really see a way to get rid of it without removing the comment from inside the macro. I put this TODO there in the first place a Luis' request, and I put it in the body of the macro because this macro already had a kernel-doc comment and I didn't think that an implementation detail TODO belonged in the user documentation. > Go ahead fix these. It appears there are few lines that either longer > than 80. In general, I keep them around 75, so it is easier read. Sorry, the above is the only checkpatch warning other than the reminder to update the MAINTAINERS file. Are you saying you want me to go through and make all the lines fit in 75 char column width? I hope not because that is going to be a pretty substantial change to make.
On 8/23/19 11:27 AM, Brendan Higgins wrote: > On Fri, Aug 23, 2019 at 10:05 AM shuah <shuah@kernel.org> wrote: >> >> On 8/23/19 10:48 AM, Brendan Higgins wrote: >>> On Fri, Aug 23, 2019 at 8:33 AM shuah <shuah@kernel.org> wrote: >>>> >>>> Hi Brendan, >>>> >>>> On 8/20/19 5:20 PM, Brendan Higgins wrote: >>>>> Add core facilities for defining unit tests; this provides a common way >>>>> to define test cases, functions that execute code which is under test >>>>> and determine whether the code under test behaves as expected; this also >>>>> provides a way to group together related test cases in test suites (here >>>>> we call them test_modules). >>>>> >>>>> Just define test cases and how to execute them for now; setting >>>>> expectations on code will be defined later. >>>>> >>>>> Signed-off-by: Brendan Higgins <brendanhiggins@google.com> >>>>> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >>>>> Reviewed-by: Logan Gunthorpe <logang@deltatee.com> >>>>> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org> >>>>> Reviewed-by: Stephen Boyd <sboyd@kernel.org> >>>>> --- >>>>> include/kunit/test.h | 179 ++++++++++++++++++++++++++++++++++++++++ >>>>> kunit/Kconfig | 17 ++++ >>>>> kunit/Makefile | 1 + >>>>> kunit/test.c | 191 +++++++++++++++++++++++++++++++++++++++++++ >>>>> 4 files changed, 388 insertions(+) >>>>> create mode 100644 include/kunit/test.h >>>>> create mode 100644 kunit/Kconfig >>>>> create mode 100644 kunit/Makefile >>>>> create mode 100644 kunit/test.c >>>>> >>>>> diff --git a/include/kunit/test.h b/include/kunit/test.h >>>>> new file mode 100644 >>>>> index 0000000000000..e0b34acb9ee4e >>>>> --- /dev/null >>>>> +++ b/include/kunit/test.h >>>>> @@ -0,0 +1,179 @@ >>>>> +/* SPDX-License-Identifier: GPL-2.0 */ >>>>> +/* >>>>> + * Base unit test (KUnit) API. >>>>> + * >>>>> + * Copyright (C) 2019, Google LLC. >>>>> + * Author: Brendan Higgins <brendanhiggins@google.com> >>>>> + */ >>>>> + >>>>> +#ifndef _KUNIT_TEST_H >>>>> +#define _KUNIT_TEST_H >>>>> + >>>>> +#include <linux/types.h> >>>>> + >>>>> +struct kunit; >>>>> + >>>>> +/** >>>>> + * struct kunit_case - represents an individual test case. >>>>> + * @run_case: the function representing the actual test case. >>>>> + * @name: the name of the test case. >>>>> + * >>>>> + * A test case is a function with the signature, ``void (*)(struct kunit *)`` >>>>> + * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. Each >>>>> + * test case is associated with a &struct kunit_suite and will be run after the >>>>> + * suite's init function and followed by the suite's exit function. >>>>> + * >>>>> + * A test case should be static and should only be created with the KUNIT_CASE() >>>>> + * macro; additionally, every array of test cases should be terminated with an >>>>> + * empty test case. >>>>> + * >>>>> + * Example: >>>> >>>> Can you fix these line continuations. It makes it very hard to read. >>>> Sorry for this late comment. These comments lines are longer than 80 >>>> and wrap. >>> >>> None of the lines in this commit are over 80 characters in column >>> width. Some are exactly 80 characters (like above). >>> >>> My guess is that you are seeing the diff added text (+ ), which when >>> you add that to a line which is exactly 80 char in length ends up >>> being over 80 char in email. If you apply the patch you will see that >>> they are only 80 chars. >>> >>>> >>>> There are several comment lines in the file that are way too long. >>> >>> Note that checkpatch also does not complain about any over 80 char >>> lines in this file. >>> >>> Sorry if I am misunderstanding what you are trying to tell me. Please >>> confirm either way. >>> >> >> WARNING: Avoid unnecessary line continuations >> #258: FILE: include/kunit/test.h:137: >> + */ \ >> >> total: 0 errors, 2 warnings, 388 lines checked > > Ah, okay so you don't like the warning about the line continuation. > That's not because it is over 80 char, but because there is a line > continuation after a comment. I don't really see a way to get rid of > it without removing the comment from inside the macro. > > I put this TODO there in the first place a Luis' request, and I put it > in the body of the macro because this macro already had a kernel-doc > comment and I didn't think that an implementation detail TODO belonged > in the user documentation. > >> Go ahead fix these. It appears there are few lines that either longer >> than 80. In general, I keep them around 75, so it is easier read. > > Sorry, the above is the only checkpatch warning other than the > reminder to update the MAINTAINERS file. > > Are you saying you want me to go through and make all the lines fit in > 75 char column width? I hope not because that is going to be a pretty > substantial change to make. > There are two things with these comment lines. One is checkpatch complaining and the other is general readability. Please go ahead and adjust them. thanks, -- Shuah
On Fri, Aug 23, 2019 at 10:34 AM shuah <shuah@kernel.org> wrote: > > On 8/23/19 11:27 AM, Brendan Higgins wrote: > > On Fri, Aug 23, 2019 at 10:05 AM shuah <shuah@kernel.org> wrote: > >> > >> On 8/23/19 10:48 AM, Brendan Higgins wrote: > >>> On Fri, Aug 23, 2019 at 8:33 AM shuah <shuah@kernel.org> wrote: > >>>> > >>>> Hi Brendan, > >>>> > >>>> On 8/20/19 5:20 PM, Brendan Higgins wrote: > >>>>> Add core facilities for defining unit tests; this provides a common way > >>>>> to define test cases, functions that execute code which is under test > >>>>> and determine whether the code under test behaves as expected; this also > >>>>> provides a way to group together related test cases in test suites (here > >>>>> we call them test_modules). > >>>>> > >>>>> Just define test cases and how to execute them for now; setting > >>>>> expectations on code will be defined later. > >>>>> > >>>>> Signed-off-by: Brendan Higgins <brendanhiggins@google.com> > >>>>> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > >>>>> Reviewed-by: Logan Gunthorpe <logang@deltatee.com> > >>>>> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org> > >>>>> Reviewed-by: Stephen Boyd <sboyd@kernel.org> > >>>>> --- > >>>>> include/kunit/test.h | 179 ++++++++++++++++++++++++++++++++++++++++ > >>>>> kunit/Kconfig | 17 ++++ > >>>>> kunit/Makefile | 1 + > >>>>> kunit/test.c | 191 +++++++++++++++++++++++++++++++++++++++++++ > >>>>> 4 files changed, 388 insertions(+) > >>>>> create mode 100644 include/kunit/test.h > >>>>> create mode 100644 kunit/Kconfig > >>>>> create mode 100644 kunit/Makefile > >>>>> create mode 100644 kunit/test.c > >>>>> > >>>>> diff --git a/include/kunit/test.h b/include/kunit/test.h > >>>>> new file mode 100644 > >>>>> index 0000000000000..e0b34acb9ee4e > >>>>> --- /dev/null > >>>>> +++ b/include/kunit/test.h > >>>>> @@ -0,0 +1,179 @@ > >>>>> +/* SPDX-License-Identifier: GPL-2.0 */ > >>>>> +/* > >>>>> + * Base unit test (KUnit) API. > >>>>> + * > >>>>> + * Copyright (C) 2019, Google LLC. > >>>>> + * Author: Brendan Higgins <brendanhiggins@google.com> > >>>>> + */ > >>>>> + > >>>>> +#ifndef _KUNIT_TEST_H > >>>>> +#define _KUNIT_TEST_H > >>>>> + > >>>>> +#include <linux/types.h> > >>>>> + > >>>>> +struct kunit; > >>>>> + > >>>>> +/** > >>>>> + * struct kunit_case - represents an individual test case. > >>>>> + * @run_case: the function representing the actual test case. > >>>>> + * @name: the name of the test case. > >>>>> + * > >>>>> + * A test case is a function with the signature, ``void (*)(struct kunit *)`` > >>>>> + * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. Each > >>>>> + * test case is associated with a &struct kunit_suite and will be run after the > >>>>> + * suite's init function and followed by the suite's exit function. > >>>>> + * > >>>>> + * A test case should be static and should only be created with the KUNIT_CASE() > >>>>> + * macro; additionally, every array of test cases should be terminated with an > >>>>> + * empty test case. > >>>>> + * > >>>>> + * Example: > >>>> > >>>> Can you fix these line continuations. It makes it very hard to read. > >>>> Sorry for this late comment. These comments lines are longer than 80 > >>>> and wrap. > >>> > >>> None of the lines in this commit are over 80 characters in column > >>> width. Some are exactly 80 characters (like above). > >>> > >>> My guess is that you are seeing the diff added text (+ ), which when > >>> you add that to a line which is exactly 80 char in length ends up > >>> being over 80 char in email. If you apply the patch you will see that > >>> they are only 80 chars. > >>> > >>>> > >>>> There are several comment lines in the file that are way too long. > >>> > >>> Note that checkpatch also does not complain about any over 80 char > >>> lines in this file. > >>> > >>> Sorry if I am misunderstanding what you are trying to tell me. Please > >>> confirm either way. > >>> > >> > >> WARNING: Avoid unnecessary line continuations > >> #258: FILE: include/kunit/test.h:137: > >> + */ \ > >> > >> total: 0 errors, 2 warnings, 388 lines checked > > > > Ah, okay so you don't like the warning about the line continuation. > > That's not because it is over 80 char, but because there is a line > > continuation after a comment. I don't really see a way to get rid of > > it without removing the comment from inside the macro. > > > > I put this TODO there in the first place a Luis' request, and I put it > > in the body of the macro because this macro already had a kernel-doc > > comment and I didn't think that an implementation detail TODO belonged > > in the user documentation. > > > >> Go ahead fix these. It appears there are few lines that either longer > >> than 80. In general, I keep them around 75, so it is easier read. > > > > Sorry, the above is the only checkpatch warning other than the > > reminder to update the MAINTAINERS file. > > > > Are you saying you want me to go through and make all the lines fit in > > 75 char column width? I hope not because that is going to be a pretty > > substantial change to make. > > > > There are two things with these comment lines. One is checkpatch > complaining and the other is general readability. So for the checkpatch warning, do you want me to move the comment out of the macro body into the kernel-doc comment? I don't really think it is the right place for a comment of this nature, but I think it is probably better than dropping it entirely (I don't see how else to do it without just removing the comment entirely). As for general readability, are you asking me to readjust all my code in all 18 patches to fit in 75 chars? Sorry for the confusion, I am just really surprised by this request. I thought the policy is 80 char, and reflowing all of my code in this patchset to 75 chars is not a quick and easy thing to do. Additionally, there are some other short term and long term issues about enforcing a 75 char limit on the KUnit code. Sorry, maybe I am just not understanding what you are asking me.
On 8/23/19 11:54 AM, Brendan Higgins wrote: > On Fri, Aug 23, 2019 at 10:34 AM shuah <shuah@kernel.org> wrote: >> >> On 8/23/19 11:27 AM, Brendan Higgins wrote: >>> On Fri, Aug 23, 2019 at 10:05 AM shuah <shuah@kernel.org> wrote: >>>> >>>> On 8/23/19 10:48 AM, Brendan Higgins wrote: >>>>> On Fri, Aug 23, 2019 at 8:33 AM shuah <shuah@kernel.org> wrote: >>>>>> >>>>>> Hi Brendan, >>>>>> >>>>>> On 8/20/19 5:20 PM, Brendan Higgins wrote: >>>>>>> Add core facilities for defining unit tests; this provides a common way >>>>>>> to define test cases, functions that execute code which is under test >>>>>>> and determine whether the code under test behaves as expected; this also >>>>>>> provides a way to group together related test cases in test suites (here >>>>>>> we call them test_modules). >>>>>>> >>>>>>> Just define test cases and how to execute them for now; setting >>>>>>> expectations on code will be defined later. >>>>>>> >>>>>>> Signed-off-by: Brendan Higgins <brendanhiggins@google.com> >>>>>>> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >>>>>>> Reviewed-by: Logan Gunthorpe <logang@deltatee.com> >>>>>>> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org> >>>>>>> Reviewed-by: Stephen Boyd <sboyd@kernel.org> >>>>>>> --- >>>>>>> include/kunit/test.h | 179 ++++++++++++++++++++++++++++++++++++++++ >>>>>>> kunit/Kconfig | 17 ++++ >>>>>>> kunit/Makefile | 1 + >>>>>>> kunit/test.c | 191 +++++++++++++++++++++++++++++++++++++++++++ >>>>>>> 4 files changed, 388 insertions(+) >>>>>>> create mode 100644 include/kunit/test.h >>>>>>> create mode 100644 kunit/Kconfig >>>>>>> create mode 100644 kunit/Makefile >>>>>>> create mode 100644 kunit/test.c >>>>>>> >>>>>>> diff --git a/include/kunit/test.h b/include/kunit/test.h >>>>>>> new file mode 100644 >>>>>>> index 0000000000000..e0b34acb9ee4e >>>>>>> --- /dev/null >>>>>>> +++ b/include/kunit/test.h >>>>>>> @@ -0,0 +1,179 @@ >>>>>>> +/* SPDX-License-Identifier: GPL-2.0 */ >>>>>>> +/* >>>>>>> + * Base unit test (KUnit) API. >>>>>>> + * >>>>>>> + * Copyright (C) 2019, Google LLC. >>>>>>> + * Author: Brendan Higgins <brendanhiggins@google.com> >>>>>>> + */ >>>>>>> + >>>>>>> +#ifndef _KUNIT_TEST_H >>>>>>> +#define _KUNIT_TEST_H >>>>>>> + >>>>>>> +#include <linux/types.h> >>>>>>> + >>>>>>> +struct kunit; >>>>>>> + >>>>>>> +/** >>>>>>> + * struct kunit_case - represents an individual test case. >>>>>>> + * @run_case: the function representing the actual test case. >>>>>>> + * @name: the name of the test case. >>>>>>> + * >>>>>>> + * A test case is a function with the signature, ``void (*)(struct kunit *)`` >>>>>>> + * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. Each >>>>>>> + * test case is associated with a &struct kunit_suite and will be run after the >>>>>>> + * suite's init function and followed by the suite's exit function. >>>>>>> + * >>>>>>> + * A test case should be static and should only be created with the KUNIT_CASE() >>>>>>> + * macro; additionally, every array of test cases should be terminated with an >>>>>>> + * empty test case. >>>>>>> + * >>>>>>> + * Example: >>>>>> >>>>>> Can you fix these line continuations. It makes it very hard to read. >>>>>> Sorry for this late comment. These comments lines are longer than 80 >>>>>> and wrap. >>>>> >>>>> None of the lines in this commit are over 80 characters in column >>>>> width. Some are exactly 80 characters (like above). >>>>> >>>>> My guess is that you are seeing the diff added text (+ ), which when >>>>> you add that to a line which is exactly 80 char in length ends up >>>>> being over 80 char in email. If you apply the patch you will see that >>>>> they are only 80 chars. >>>>> >>>>>> >>>>>> There are several comment lines in the file that are way too long. >>>>> >>>>> Note that checkpatch also does not complain about any over 80 char >>>>> lines in this file. >>>>> >>>>> Sorry if I am misunderstanding what you are trying to tell me. Please >>>>> confirm either way. >>>>> >>>> >>>> WARNING: Avoid unnecessary line continuations >>>> #258: FILE: include/kunit/test.h:137: >>>> + */ \ >>>> >>>> total: 0 errors, 2 warnings, 388 lines checked >>> >>> Ah, okay so you don't like the warning about the line continuation. >>> That's not because it is over 80 char, but because there is a line >>> continuation after a comment. I don't really see a way to get rid of >>> it without removing the comment from inside the macro. >>> >>> I put this TODO there in the first place a Luis' request, and I put it >>> in the body of the macro because this macro already had a kernel-doc >>> comment and I didn't think that an implementation detail TODO belonged >>> in the user documentation. >>> >>>> Go ahead fix these. It appears there are few lines that either longer >>>> than 80. In general, I keep them around 75, so it is easier read. >>> >>> Sorry, the above is the only checkpatch warning other than the >>> reminder to update the MAINTAINERS file. >>> >>> Are you saying you want me to go through and make all the lines fit in >>> 75 char column width? I hope not because that is going to be a pretty >>> substantial change to make. >>> >> >> There are two things with these comment lines. One is checkpatch >> complaining and the other is general readability. > > So for the checkpatch warning, do you want me to move the comment out > of the macro body into the kernel-doc comment? I don't really think it > is the right place for a comment of this nature, but I think it is > probably better than dropping it entirely (I don't see how else to do > it without just removing the comment entirely). > Don't drop the comments. It makes perfect sense to turn this into a kernel-doc comment. We are going back forth on this a lot. I see several lines 81+ in this file. I am at 5.3-rc5 and my commit hooks aren't happy. I am fine with it if you want to convert these to kernel-doc comments. I think it makes perfect sense. I would like it if you take a look and fix the lines that are longer than 80. If you think you would rather spend time converting them to kernel-doc, I am fine with it. thanks, -- Shuah
On Fri, Aug 23, 2019 at 11:32 AM shuah <shuah@kernel.org> wrote: > > On 8/23/19 11:54 AM, Brendan Higgins wrote: > > On Fri, Aug 23, 2019 at 10:34 AM shuah <shuah@kernel.org> wrote: > >> > >> On 8/23/19 11:27 AM, Brendan Higgins wrote: > >>> On Fri, Aug 23, 2019 at 10:05 AM shuah <shuah@kernel.org> wrote: > >>>> > >>>> On 8/23/19 10:48 AM, Brendan Higgins wrote: > >>>>> On Fri, Aug 23, 2019 at 8:33 AM shuah <shuah@kernel.org> wrote: > >>>>>> > >>>>>> Hi Brendan, > >>>>>> > >>>>>> On 8/20/19 5:20 PM, Brendan Higgins wrote: > >>>>>>> Add core facilities for defining unit tests; this provides a common way > >>>>>>> to define test cases, functions that execute code which is under test > >>>>>>> and determine whether the code under test behaves as expected; this also > >>>>>>> provides a way to group together related test cases in test suites (here > >>>>>>> we call them test_modules). > >>>>>>> > >>>>>>> Just define test cases and how to execute them for now; setting > >>>>>>> expectations on code will be defined later. > >>>>>>> > >>>>>>> Signed-off-by: Brendan Higgins <brendanhiggins@google.com> > >>>>>>> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > >>>>>>> Reviewed-by: Logan Gunthorpe <logang@deltatee.com> > >>>>>>> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org> > >>>>>>> Reviewed-by: Stephen Boyd <sboyd@kernel.org> > >>>>>>> --- > >>>>>>> include/kunit/test.h | 179 ++++++++++++++++++++++++++++++++++++++++ > >>>>>>> kunit/Kconfig | 17 ++++ > >>>>>>> kunit/Makefile | 1 + > >>>>>>> kunit/test.c | 191 +++++++++++++++++++++++++++++++++++++++++++ > >>>>>>> 4 files changed, 388 insertions(+) > >>>>>>> create mode 100644 include/kunit/test.h > >>>>>>> create mode 100644 kunit/Kconfig > >>>>>>> create mode 100644 kunit/Makefile > >>>>>>> create mode 100644 kunit/test.c > >>>>>>> > >>>>>>> diff --git a/include/kunit/test.h b/include/kunit/test.h > >>>>>>> new file mode 100644 > >>>>>>> index 0000000000000..e0b34acb9ee4e > >>>>>>> --- /dev/null > >>>>>>> +++ b/include/kunit/test.h > >>>>>>> @@ -0,0 +1,179 @@ > >>>>>>> +/* SPDX-License-Identifier: GPL-2.0 */ > >>>>>>> +/* > >>>>>>> + * Base unit test (KUnit) API. > >>>>>>> + * > >>>>>>> + * Copyright (C) 2019, Google LLC. > >>>>>>> + * Author: Brendan Higgins <brendanhiggins@google.com> > >>>>>>> + */ > >>>>>>> + > >>>>>>> +#ifndef _KUNIT_TEST_H > >>>>>>> +#define _KUNIT_TEST_H > >>>>>>> + > >>>>>>> +#include <linux/types.h> > >>>>>>> + > >>>>>>> +struct kunit; > >>>>>>> + > >>>>>>> +/** > >>>>>>> + * struct kunit_case - represents an individual test case. > >>>>>>> + * @run_case: the function representing the actual test case. > >>>>>>> + * @name: the name of the test case. > >>>>>>> + * > >>>>>>> + * A test case is a function with the signature, ``void (*)(struct kunit *)`` > >>>>>>> + * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. Each > >>>>>>> + * test case is associated with a &struct kunit_suite and will be run after the > >>>>>>> + * suite's init function and followed by the suite's exit function. > >>>>>>> + * > >>>>>>> + * A test case should be static and should only be created with the KUNIT_CASE() > >>>>>>> + * macro; additionally, every array of test cases should be terminated with an > >>>>>>> + * empty test case. > >>>>>>> + * > >>>>>>> + * Example: > >>>>>> > >>>>>> Can you fix these line continuations. It makes it very hard to read. > >>>>>> Sorry for this late comment. These comments lines are longer than 80 > >>>>>> and wrap. > >>>>> > >>>>> None of the lines in this commit are over 80 characters in column > >>>>> width. Some are exactly 80 characters (like above). > >>>>> > >>>>> My guess is that you are seeing the diff added text (+ ), which when > >>>>> you add that to a line which is exactly 80 char in length ends up > >>>>> being over 80 char in email. If you apply the patch you will see that > >>>>> they are only 80 chars. > >>>>> > >>>>>> > >>>>>> There are several comment lines in the file that are way too long. > >>>>> > >>>>> Note that checkpatch also does not complain about any over 80 char > >>>>> lines in this file. > >>>>> > >>>>> Sorry if I am misunderstanding what you are trying to tell me. Please > >>>>> confirm either way. > >>>>> > >>>> > >>>> WARNING: Avoid unnecessary line continuations > >>>> #258: FILE: include/kunit/test.h:137: > >>>> + */ \ > >>>> > >>>> total: 0 errors, 2 warnings, 388 lines checked > >>> > >>> Ah, okay so you don't like the warning about the line continuation. > >>> That's not because it is over 80 char, but because there is a line > >>> continuation after a comment. I don't really see a way to get rid of > >>> it without removing the comment from inside the macro. > >>> > >>> I put this TODO there in the first place a Luis' request, and I put it > >>> in the body of the macro because this macro already had a kernel-doc > >>> comment and I didn't think that an implementation detail TODO belonged > >>> in the user documentation. > >>> > >>>> Go ahead fix these. It appears there are few lines that either longer > >>>> than 80. In general, I keep them around 75, so it is easier read. > >>> > >>> Sorry, the above is the only checkpatch warning other than the > >>> reminder to update the MAINTAINERS file. > >>> > >>> Are you saying you want me to go through and make all the lines fit in > >>> 75 char column width? I hope not because that is going to be a pretty > >>> substantial change to make. > >>> > >> > >> There are two things with these comment lines. One is checkpatch > >> complaining and the other is general readability. > > > > So for the checkpatch warning, do you want me to move the comment out > > of the macro body into the kernel-doc comment? I don't really think it > > is the right place for a comment of this nature, but I think it is > > probably better than dropping it entirely (I don't see how else to do > > it without just removing the comment entirely). > > > > Don't drop the comments. It makes perfect sense to turn this into a > kernel-doc comment. I am fine with that. I will do that in a subsequent revision once we figure out the column limit issue. > We are going back forth on this a lot. I see several lines 81+ in > this file. I am at 5.3-rc5 and my commit hooks aren't happy. I am > fine with it if you want to convert these to kernel-doc comments. > I think it makes perfect sense. Okay, so this is interesting. When I look at the applied patches in my local repo, I don't see any 81+ lines. So it seems that something interesting is going on here. To be clear (sorry for the stupid question) you are seeing the issue after you applied the patch, and not in the patch file itself? Since we are still at OSS, would you mind if we meet up this afternoon so I can see this issue you are seeing? I imagine we should get this figured out pretty quickly. Cheers
On 8/23/19 12:56 PM, Brendan Higgins wrote: > On Fri, Aug 23, 2019 at 11:32 AM shuah <shuah@kernel.org> wrote: >> >> On 8/23/19 11:54 AM, Brendan Higgins wrote: >>> On Fri, Aug 23, 2019 at 10:34 AM shuah <shuah@kernel.org> wrote: >>>> >>>> On 8/23/19 11:27 AM, Brendan Higgins wrote: >>>>> On Fri, Aug 23, 2019 at 10:05 AM shuah <shuah@kernel.org> wrote: >>>>>> >>>>>> On 8/23/19 10:48 AM, Brendan Higgins wrote: >>>>>>> On Fri, Aug 23, 2019 at 8:33 AM shuah <shuah@kernel.org> wrote: >>>>>>>> >>>>>>>> Hi Brendan, >>>>>>>> >>>>>>>> On 8/20/19 5:20 PM, Brendan Higgins wrote: >>>>>>>>> Add core facilities for defining unit tests; this provides a common way >>>>>>>>> to define test cases, functions that execute code which is under test >>>>>>>>> and determine whether the code under test behaves as expected; this also >>>>>>>>> provides a way to group together related test cases in test suites (here >>>>>>>>> we call them test_modules). >>>>>>>>> >>>>>>>>> Just define test cases and how to execute them for now; setting >>>>>>>>> expectations on code will be defined later. >>>>>>>>> >>>>>>>>> Signed-off-by: Brendan Higgins <brendanhiggins@google.com> >>>>>>>>> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >>>>>>>>> Reviewed-by: Logan Gunthorpe <logang@deltatee.com> >>>>>>>>> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org> >>>>>>>>> Reviewed-by: Stephen Boyd <sboyd@kernel.org> >>>>>>>>> --- >>>>>>>>> include/kunit/test.h | 179 ++++++++++++++++++++++++++++++++++++++++ >>>>>>>>> kunit/Kconfig | 17 ++++ >>>>>>>>> kunit/Makefile | 1 + >>>>>>>>> kunit/test.c | 191 +++++++++++++++++++++++++++++++++++++++++++ >>>>>>>>> 4 files changed, 388 insertions(+) >>>>>>>>> create mode 100644 include/kunit/test.h >>>>>>>>> create mode 100644 kunit/Kconfig >>>>>>>>> create mode 100644 kunit/Makefile >>>>>>>>> create mode 100644 kunit/test.c >>>>>>>>> >>>>>>>>> diff --git a/include/kunit/test.h b/include/kunit/test.h >>>>>>>>> new file mode 100644 >>>>>>>>> index 0000000000000..e0b34acb9ee4e >>>>>>>>> --- /dev/null >>>>>>>>> +++ b/include/kunit/test.h >>>>>>>>> @@ -0,0 +1,179 @@ >>>>>>>>> +/* SPDX-License-Identifier: GPL-2.0 */ >>>>>>>>> +/* >>>>>>>>> + * Base unit test (KUnit) API. >>>>>>>>> + * >>>>>>>>> + * Copyright (C) 2019, Google LLC. >>>>>>>>> + * Author: Brendan Higgins <brendanhiggins@google.com> >>>>>>>>> + */ >>>>>>>>> + >>>>>>>>> +#ifndef _KUNIT_TEST_H >>>>>>>>> +#define _KUNIT_TEST_H >>>>>>>>> + >>>>>>>>> +#include <linux/types.h> >>>>>>>>> + >>>>>>>>> +struct kunit; >>>>>>>>> + >>>>>>>>> +/** >>>>>>>>> + * struct kunit_case - represents an individual test case. >>>>>>>>> + * @run_case: the function representing the actual test case. >>>>>>>>> + * @name: the name of the test case. >>>>>>>>> + * >>>>>>>>> + * A test case is a function with the signature, ``void (*)(struct kunit *)`` >>>>>>>>> + * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. Each >>>>>>>>> + * test case is associated with a &struct kunit_suite and will be run after the >>>>>>>>> + * suite's init function and followed by the suite's exit function. >>>>>>>>> + * >>>>>>>>> + * A test case should be static and should only be created with the KUNIT_CASE() >>>>>>>>> + * macro; additionally, every array of test cases should be terminated with an >>>>>>>>> + * empty test case. >>>>>>>>> + * >>>>>>>>> + * Example: >>>>>>>> >>>>>>>> Can you fix these line continuations. It makes it very hard to read. >>>>>>>> Sorry for this late comment. These comments lines are longer than 80 >>>>>>>> and wrap. >>>>>>> >>>>>>> None of the lines in this commit are over 80 characters in column >>>>>>> width. Some are exactly 80 characters (like above). >>>>>>> >>>>>>> My guess is that you are seeing the diff added text (+ ), which when >>>>>>> you add that to a line which is exactly 80 char in length ends up >>>>>>> being over 80 char in email. If you apply the patch you will see that >>>>>>> they are only 80 chars. >>>>>>> >>>>>>>> >>>>>>>> There are several comment lines in the file that are way too long. >>>>>>> >>>>>>> Note that checkpatch also does not complain about any over 80 char >>>>>>> lines in this file. >>>>>>> >>>>>>> Sorry if I am misunderstanding what you are trying to tell me. Please >>>>>>> confirm either way. >>>>>>> >>>>>> >>>>>> WARNING: Avoid unnecessary line continuations >>>>>> #258: FILE: include/kunit/test.h:137: >>>>>> + */ \ >>>>>> >>>>>> total: 0 errors, 2 warnings, 388 lines checked >>>>> >>>>> Ah, okay so you don't like the warning about the line continuation. >>>>> That's not because it is over 80 char, but because there is a line >>>>> continuation after a comment. I don't really see a way to get rid of >>>>> it without removing the comment from inside the macro. >>>>> >>>>> I put this TODO there in the first place a Luis' request, and I put it >>>>> in the body of the macro because this macro already had a kernel-doc >>>>> comment and I didn't think that an implementation detail TODO belonged >>>>> in the user documentation. >>>>> >>>>>> Go ahead fix these. It appears there are few lines that either longer >>>>>> than 80. In general, I keep them around 75, so it is easier read. >>>>> >>>>> Sorry, the above is the only checkpatch warning other than the >>>>> reminder to update the MAINTAINERS file. >>>>> >>>>> Are you saying you want me to go through and make all the lines fit in >>>>> 75 char column width? I hope not because that is going to be a pretty >>>>> substantial change to make. >>>>> >>>> >>>> There are two things with these comment lines. One is checkpatch >>>> complaining and the other is general readability. >>> >>> So for the checkpatch warning, do you want me to move the comment out >>> of the macro body into the kernel-doc comment? I don't really think it >>> is the right place for a comment of this nature, but I think it is >>> probably better than dropping it entirely (I don't see how else to do >>> it without just removing the comment entirely). >>> >> >> Don't drop the comments. It makes perfect sense to turn this into a >> kernel-doc comment. > > I am fine with that. I will do that in a subsequent revision once we > figure out the column limit issue. > >> We are going back forth on this a lot. I see several lines 81+ in >> this file. I am at 5.3-rc5 and my commit hooks aren't happy. I am >> fine with it if you want to convert these to kernel-doc comments. >> I think it makes perfect sense. > > Okay, so this is interesting. When I look at the applied patches in my > local repo, I don't see any 81+ lines. So it seems that something > interesting is going on here. > > To be clear (sorry for the stupid question) you are seeing the issue > after you applied the patch, and not in the patch file itself? > I am using my normal workflow. My pre-commit check is catching this. Just this patch. All others are good other than the 9/18 BUG() issue. > Since we are still at OSS, would you mind if we meet up this afternoon > so I can see this issue you are seeing? I imagine we should get this > figured out pretty quickly. > Yeah. Would have been nice. I am not at oss today. thanks, -- Shuah
On Fri, Aug 23, 2019 at 12:04 PM shuah <shuah@kernel.org> wrote: > > On 8/23/19 12:56 PM, Brendan Higgins wrote: > > On Fri, Aug 23, 2019 at 11:32 AM shuah <shuah@kernel.org> wrote: > >> > >> On 8/23/19 11:54 AM, Brendan Higgins wrote: > >>> On Fri, Aug 23, 2019 at 10:34 AM shuah <shuah@kernel.org> wrote: > >>>> > >>>> On 8/23/19 11:27 AM, Brendan Higgins wrote: > >>>>> On Fri, Aug 23, 2019 at 10:05 AM shuah <shuah@kernel.org> wrote: > >>>>>> > >>>>>> On 8/23/19 10:48 AM, Brendan Higgins wrote: > >>>>>>> On Fri, Aug 23, 2019 at 8:33 AM shuah <shuah@kernel.org> wrote: > >>>>>>>> > >>>>>>>> Hi Brendan, > >>>>>>>> > >>>>>>>> On 8/20/19 5:20 PM, Brendan Higgins wrote: > >>>>>>>>> Add core facilities for defining unit tests; this provides a common way > >>>>>>>>> to define test cases, functions that execute code which is under test > >>>>>>>>> and determine whether the code under test behaves as expected; this also > >>>>>>>>> provides a way to group together related test cases in test suites (here > >>>>>>>>> we call them test_modules). > >>>>>>>>> > >>>>>>>>> Just define test cases and how to execute them for now; setting > >>>>>>>>> expectations on code will be defined later. > >>>>>>>>> > >>>>>>>>> Signed-off-by: Brendan Higgins <brendanhiggins@google.com> > >>>>>>>>> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > >>>>>>>>> Reviewed-by: Logan Gunthorpe <logang@deltatee.com> > >>>>>>>>> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org> > >>>>>>>>> Reviewed-by: Stephen Boyd <sboyd@kernel.org> > >>>>>>>>> --- > >>>>>>>>> include/kunit/test.h | 179 ++++++++++++++++++++++++++++++++++++++++ > >>>>>>>>> kunit/Kconfig | 17 ++++ > >>>>>>>>> kunit/Makefile | 1 + > >>>>>>>>> kunit/test.c | 191 +++++++++++++++++++++++++++++++++++++++++++ > >>>>>>>>> 4 files changed, 388 insertions(+) > >>>>>>>>> create mode 100644 include/kunit/test.h > >>>>>>>>> create mode 100644 kunit/Kconfig > >>>>>>>>> create mode 100644 kunit/Makefile > >>>>>>>>> create mode 100644 kunit/test.c > >>>>>>>>> > >>>>>>>>> diff --git a/include/kunit/test.h b/include/kunit/test.h > >>>>>>>>> new file mode 100644 > >>>>>>>>> index 0000000000000..e0b34acb9ee4e > >>>>>>>>> --- /dev/null > >>>>>>>>> +++ b/include/kunit/test.h > >>>>>>>>> @@ -0,0 +1,179 @@ > >>>>>>>>> +/* SPDX-License-Identifier: GPL-2.0 */ > >>>>>>>>> +/* > >>>>>>>>> + * Base unit test (KUnit) API. > >>>>>>>>> + * > >>>>>>>>> + * Copyright (C) 2019, Google LLC. > >>>>>>>>> + * Author: Brendan Higgins <brendanhiggins@google.com> > >>>>>>>>> + */ > >>>>>>>>> + > >>>>>>>>> +#ifndef _KUNIT_TEST_H > >>>>>>>>> +#define _KUNIT_TEST_H > >>>>>>>>> + > >>>>>>>>> +#include <linux/types.h> > >>>>>>>>> + > >>>>>>>>> +struct kunit; > >>>>>>>>> + > >>>>>>>>> +/** > >>>>>>>>> + * struct kunit_case - represents an individual test case. > >>>>>>>>> + * @run_case: the function representing the actual test case. > >>>>>>>>> + * @name: the name of the test case. > >>>>>>>>> + * > >>>>>>>>> + * A test case is a function with the signature, ``void (*)(struct kunit *)`` > >>>>>>>>> + * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. Each > >>>>>>>>> + * test case is associated with a &struct kunit_suite and will be run after the > >>>>>>>>> + * suite's init function and followed by the suite's exit function. > >>>>>>>>> + * > >>>>>>>>> + * A test case should be static and should only be created with the KUNIT_CASE() > >>>>>>>>> + * macro; additionally, every array of test cases should be terminated with an > >>>>>>>>> + * empty test case. > >>>>>>>>> + * > >>>>>>>>> + * Example: > >>>>>>>> > >>>>>>>> Can you fix these line continuations. It makes it very hard to read. > >>>>>>>> Sorry for this late comment. These comments lines are longer than 80 > >>>>>>>> and wrap. > >>>>>>> > >>>>>>> None of the lines in this commit are over 80 characters in column > >>>>>>> width. Some are exactly 80 characters (like above). > >>>>>>> > >>>>>>> My guess is that you are seeing the diff added text (+ ), which when > >>>>>>> you add that to a line which is exactly 80 char in length ends up > >>>>>>> being over 80 char in email. If you apply the patch you will see that > >>>>>>> they are only 80 chars. > >>>>>>> > >>>>>>>> > >>>>>>>> There are several comment lines in the file that are way too long. > >>>>>>> > >>>>>>> Note that checkpatch also does not complain about any over 80 char > >>>>>>> lines in this file. > >>>>>>> > >>>>>>> Sorry if I am misunderstanding what you are trying to tell me. Please > >>>>>>> confirm either way. > >>>>>>> > >>>>>> > >>>>>> WARNING: Avoid unnecessary line continuations > >>>>>> #258: FILE: include/kunit/test.h:137: > >>>>>> + */ \ > >>>>>> > >>>>>> total: 0 errors, 2 warnings, 388 lines checked > >>>>> > >>>>> Ah, okay so you don't like the warning about the line continuation. > >>>>> That's not because it is over 80 char, but because there is a line > >>>>> continuation after a comment. I don't really see a way to get rid of > >>>>> it without removing the comment from inside the macro. > >>>>> > >>>>> I put this TODO there in the first place a Luis' request, and I put it > >>>>> in the body of the macro because this macro already had a kernel-doc > >>>>> comment and I didn't think that an implementation detail TODO belonged > >>>>> in the user documentation. > >>>>> > >>>>>> Go ahead fix these. It appears there are few lines that either longer > >>>>>> than 80. In general, I keep them around 75, so it is easier read. > >>>>> > >>>>> Sorry, the above is the only checkpatch warning other than the > >>>>> reminder to update the MAINTAINERS file. > >>>>> > >>>>> Are you saying you want me to go through and make all the lines fit in > >>>>> 75 char column width? I hope not because that is going to be a pretty > >>>>> substantial change to make. > >>>>> > >>>> > >>>> There are two things with these comment lines. One is checkpatch > >>>> complaining and the other is general readability. > >>> > >>> So for the checkpatch warning, do you want me to move the comment out > >>> of the macro body into the kernel-doc comment? I don't really think it > >>> is the right place for a comment of this nature, but I think it is > >>> probably better than dropping it entirely (I don't see how else to do > >>> it without just removing the comment entirely). > >>> > >> > >> Don't drop the comments. It makes perfect sense to turn this into a > >> kernel-doc comment. > > > > I am fine with that. I will do that in a subsequent revision once we > > figure out the column limit issue. > > > >> We are going back forth on this a lot. I see several lines 81+ in > >> this file. I am at 5.3-rc5 and my commit hooks aren't happy. I am > >> fine with it if you want to convert these to kernel-doc comments. > >> I think it makes perfect sense. > > > > Okay, so this is interesting. When I look at the applied patches in my > > local repo, I don't see any 81+ lines. So it seems that something > > interesting is going on here. > > > > To be clear (sorry for the stupid question) you are seeing the issue > > after you applied the patch, and not in the patch file itself? > > > > I am using my normal workflow. My pre-commit check is catching this. > Just this patch. Okay, *that* is super strange! So I have lines in this patch (01/18) that are exactly 80 char wide and I was thinking that it might be an off by one issue on either my workflow or your workflow, but I have lines in other patches that are exactly 80 char wide and our setups agree that they are fine, so I really am not sure what's going on here. It sounds like you are only seeing the issue in only a couple places, do you mind calling out the specific lines that are problematic? > All others are good other than the 9/18 BUG() issue. > > Since we are still at OSS, would you mind if we meet up this afternoon > > so I can see this issue you are seeing? I imagine we should get this > > figured out pretty quickly. > > > > Yeah. Would have been nice. I am not at oss today. Dang.
On 8/23/19 1:20 PM, Brendan Higgins wrote: > On Fri, Aug 23, 2019 at 12:04 PM shuah <shuah@kernel.org> wrote: >> >> On 8/23/19 12:56 PM, Brendan Higgins wrote: >>> On Fri, Aug 23, 2019 at 11:32 AM shuah <shuah@kernel.org> wrote: >>>> >>>> On 8/23/19 11:54 AM, Brendan Higgins wrote: >>>>> On Fri, Aug 23, 2019 at 10:34 AM shuah <shuah@kernel.org> wrote: >>>>>> >>>>>> On 8/23/19 11:27 AM, Brendan Higgins wrote: >>>>>>> On Fri, Aug 23, 2019 at 10:05 AM shuah <shuah@kernel.org> wrote: >>>>>>>> >>>>>>>> On 8/23/19 10:48 AM, Brendan Higgins wrote: >>>>>>>>> On Fri, Aug 23, 2019 at 8:33 AM shuah <shuah@kernel.org> wrote: >>>>>>>>>> >>>>>>>>>> Hi Brendan, >>>>>>>>>> >>>>>>>>>> On 8/20/19 5:20 PM, Brendan Higgins wrote: >>>>>>>>>>> Add core facilities for defining unit tests; this provides a common way >>>>>>>>>>> to define test cases, functions that execute code which is under test >>>>>>>>>>> and determine whether the code under test behaves as expected; this also >>>>>>>>>>> provides a way to group together related test cases in test suites (here >>>>>>>>>>> we call them test_modules). >>>>>>>>>>> >>>>>>>>>>> Just define test cases and how to execute them for now; setting >>>>>>>>>>> expectations on code will be defined later. >>>>>>>>>>> >>>>>>>>>>> Signed-off-by: Brendan Higgins <brendanhiggins@google.com> >>>>>>>>>>> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >>>>>>>>>>> Reviewed-by: Logan Gunthorpe <logang@deltatee.com> >>>>>>>>>>> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org> >>>>>>>>>>> Reviewed-by: Stephen Boyd <sboyd@kernel.org> >>>>>>>>>>> --- >>>>>>>>>>> include/kunit/test.h | 179 ++++++++++++++++++++++++++++++++++++++++ >>>>>>>>>>> kunit/Kconfig | 17 ++++ >>>>>>>>>>> kunit/Makefile | 1 + >>>>>>>>>>> kunit/test.c | 191 +++++++++++++++++++++++++++++++++++++++++++ >>>>>>>>>>> 4 files changed, 388 insertions(+) >>>>>>>>>>> create mode 100644 include/kunit/test.h >>>>>>>>>>> create mode 100644 kunit/Kconfig >>>>>>>>>>> create mode 100644 kunit/Makefile >>>>>>>>>>> create mode 100644 kunit/test.c >>>>>>>>>>> >>>>>>>>>>> diff --git a/include/kunit/test.h b/include/kunit/test.h >>>>>>>>>>> new file mode 100644 >>>>>>>>>>> index 0000000000000..e0b34acb9ee4e >>>>>>>>>>> --- /dev/null >>>>>>>>>>> +++ b/include/kunit/test.h >>>>>>>>>>> @@ -0,0 +1,179 @@ >>>>>>>>>>> +/* SPDX-License-Identifier: GPL-2.0 */ >>>>>>>>>>> +/* >>>>>>>>>>> + * Base unit test (KUnit) API. >>>>>>>>>>> + * >>>>>>>>>>> + * Copyright (C) 2019, Google LLC. >>>>>>>>>>> + * Author: Brendan Higgins <brendanhiggins@google.com> >>>>>>>>>>> + */ >>>>>>>>>>> + >>>>>>>>>>> +#ifndef _KUNIT_TEST_H >>>>>>>>>>> +#define _KUNIT_TEST_H >>>>>>>>>>> + >>>>>>>>>>> +#include <linux/types.h> >>>>>>>>>>> + >>>>>>>>>>> +struct kunit; >>>>>>>>>>> + >>>>>>>>>>> +/** >>>>>>>>>>> + * struct kunit_case - represents an individual test case. >>>>>>>>>>> + * @run_case: the function representing the actual test case. >>>>>>>>>>> + * @name: the name of the test case. >>>>>>>>>>> + * >>>>>>>>>>> + * A test case is a function with the signature, ``void (*)(struct kunit *)`` >>>>>>>>>>> + * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. Each >>>>>>>>>>> + * test case is associated with a &struct kunit_suite and will be run after the >>>>>>>>>>> + * suite's init function and followed by the suite's exit function. >>>>>>>>>>> + * >>>>>>>>>>> + * A test case should be static and should only be created with the KUNIT_CASE() >>>>>>>>>>> + * macro; additionally, every array of test cases should be terminated with an >>>>>>>>>>> + * empty test case. >>>>>>>>>>> + * >>>>>>>>>>> + * Example: >>>>>>>>>> >>>>>>>>>> Can you fix these line continuations. It makes it very hard to read. >>>>>>>>>> Sorry for this late comment. These comments lines are longer than 80 >>>>>>>>>> and wrap. >>>>>>>>> >>>>>>>>> None of the lines in this commit are over 80 characters in column >>>>>>>>> width. Some are exactly 80 characters (like above). >>>>>>>>> >>>>>>>>> My guess is that you are seeing the diff added text (+ ), which when >>>>>>>>> you add that to a line which is exactly 80 char in length ends up >>>>>>>>> being over 80 char in email. If you apply the patch you will see that >>>>>>>>> they are only 80 chars. >>>>>>>>> >>>>>>>>>> >>>>>>>>>> There are several comment lines in the file that are way too long. >>>>>>>>> >>>>>>>>> Note that checkpatch also does not complain about any over 80 char >>>>>>>>> lines in this file. >>>>>>>>> >>>>>>>>> Sorry if I am misunderstanding what you are trying to tell me. Please >>>>>>>>> confirm either way. >>>>>>>>> >>>>>>>> >>>>>>>> WARNING: Avoid unnecessary line continuations >>>>>>>> #258: FILE: include/kunit/test.h:137: >>>>>>>> + */ \ >>>>>>>> >>>>>>>> total: 0 errors, 2 warnings, 388 lines checked >>>>>>> >>>>>>> Ah, okay so you don't like the warning about the line continuation. >>>>>>> That's not because it is over 80 char, but because there is a line >>>>>>> continuation after a comment. I don't really see a way to get rid of >>>>>>> it without removing the comment from inside the macro. >>>>>>> >>>>>>> I put this TODO there in the first place a Luis' request, and I put it >>>>>>> in the body of the macro because this macro already had a kernel-doc >>>>>>> comment and I didn't think that an implementation detail TODO belonged >>>>>>> in the user documentation. >>>>>>> >>>>>>>> Go ahead fix these. It appears there are few lines that either longer >>>>>>>> than 80. In general, I keep them around 75, so it is easier read. >>>>>>> >>>>>>> Sorry, the above is the only checkpatch warning other than the >>>>>>> reminder to update the MAINTAINERS file. >>>>>>> >>>>>>> Are you saying you want me to go through and make all the lines fit in >>>>>>> 75 char column width? I hope not because that is going to be a pretty >>>>>>> substantial change to make. >>>>>>> >>>>>> >>>>>> There are two things with these comment lines. One is checkpatch >>>>>> complaining and the other is general readability. >>>>> >>>>> So for the checkpatch warning, do you want me to move the comment out >>>>> of the macro body into the kernel-doc comment? I don't really think it >>>>> is the right place for a comment of this nature, but I think it is >>>>> probably better than dropping it entirely (I don't see how else to do >>>>> it without just removing the comment entirely). >>>>> >>>> >>>> Don't drop the comments. It makes perfect sense to turn this into a >>>> kernel-doc comment. >>> >>> I am fine with that. I will do that in a subsequent revision once we >>> figure out the column limit issue. >>> >>>> We are going back forth on this a lot. I see several lines 81+ in >>>> this file. I am at 5.3-rc5 and my commit hooks aren't happy. I am >>>> fine with it if you want to convert these to kernel-doc comments. >>>> I think it makes perfect sense. >>> >>> Okay, so this is interesting. When I look at the applied patches in my >>> local repo, I don't see any 81+ lines. So it seems that something >>> interesting is going on here. >>> >>> To be clear (sorry for the stupid question) you are seeing the issue >>> after you applied the patch, and not in the patch file itself? >>> >> >> I am using my normal workflow. My pre-commit check is catching this. >> Just this patch. > > Okay, *that* is super strange! > > So I have lines in this patch (01/18) that are exactly 80 char wide > and I was thinking that it might be an off by one issue on either my > workflow or your workflow, but I have lines in other patches that are > exactly 80 char wide and our setups agree that they are fine, so I > really am not sure what's going on here. > > It sounds like you are only seeing the issue in only a couple places, > do you mind calling out the specific lines that are problematic? Take a look at the comment blocks. That is where the problem are. > >> All others are good other than the 9/18 BUG() issue. >>> Since we are still at OSS, would you mind if we meet up this afternoon >>> so I can see this issue you are seeing? I imagine we should get this >>> figured out pretty quickly. >>> >> >> Yeah. Would have been nice. I am not at oss today. > > Dang. > thanks, -- Shuah
diff --git a/include/kunit/test.h b/include/kunit/test.h new file mode 100644 index 0000000000000..e0b34acb9ee4e --- /dev/null +++ b/include/kunit/test.h @@ -0,0 +1,179 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Base unit test (KUnit) API. + * + * Copyright (C) 2019, Google LLC. + * Author: Brendan Higgins <brendanhiggins@google.com> + */ + +#ifndef _KUNIT_TEST_H +#define _KUNIT_TEST_H + +#include <linux/types.h> + +struct kunit; + +/** + * struct kunit_case - represents an individual test case. + * @run_case: the function representing the actual test case. + * @name: the name of the test case. + * + * A test case is a function with the signature, ``void (*)(struct kunit *)`` + * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. Each + * test case is associated with a &struct kunit_suite and will be run after the + * suite's init function and followed by the suite's exit function. + * + * A test case should be static and should only be created with the KUNIT_CASE() + * macro; additionally, every array of test cases should be terminated with an + * empty test case. + * + * Example: + * + * .. code-block:: c + * + * void add_test_basic(struct kunit *test) + * { + * KUNIT_EXPECT_EQ(test, 1, add(1, 0)); + * KUNIT_EXPECT_EQ(test, 2, add(1, 1)); + * KUNIT_EXPECT_EQ(test, 0, add(-1, 1)); + * KUNIT_EXPECT_EQ(test, INT_MAX, add(0, INT_MAX)); + * KUNIT_EXPECT_EQ(test, -1, add(INT_MAX, INT_MIN)); + * } + * + * static struct kunit_case example_test_cases[] = { + * KUNIT_CASE(add_test_basic), + * {} + * }; + * + */ +struct kunit_case { + void (*run_case)(struct kunit *test); + const char *name; + + /* private: internal use only. */ + bool success; +}; + +/** + * KUNIT_CASE - A helper for creating a &struct kunit_case + * @test_name: a reference to a test case function. + * + * Takes a symbol for a function representing a test case and creates a + * &struct kunit_case object from it. See the documentation for + * &struct kunit_case for an example on how to use it. + */ +#define KUNIT_CASE(test_name) { .run_case = test_name, .name = #test_name } + +/** + * struct kunit_suite - describes a related collection of &struct kunit_case s. + * @name: the name of the test. Purely informational. + * @init: called before every test case. + * @exit: called after every test case. + * @test_cases: a null terminated array of test cases. + * + * A kunit_suite is a collection of related &struct kunit_case s, such that + * @init is called before every test case and @exit is called after every test + * case, similar to the notion of a *test fixture* or a *test class* in other + * unit testing frameworks like JUnit or Googletest. + * + * Every &struct kunit_case must be associated with a kunit_suite for KUnit to + * run it. + */ +struct kunit_suite { + const char name[256]; + int (*init)(struct kunit *test); + void (*exit)(struct kunit *test); + struct kunit_case *test_cases; +}; + +/** + * struct kunit - represents a running instance of a test. + * @priv: for user to store arbitrary data. Commonly used to pass data created + * in the init function (see &struct kunit_suite). + * + * Used to store information about the current context under which the test is + * running. Most of this data is private and should only be accessed indirectly + * via public functions; the one exception is @priv which can be used by the + * test writer to store arbitrary data. + */ +struct kunit { + void *priv; + + /* private: internal use only. */ + const char *name; /* Read only after initialization! */ + /* + * success starts as true, and may only be set to false during a test + * case; thus, it is safe to update this across multiple threads using + * WRITE_ONCE; however, as a consequence, it may only be read after the + * test case finishes once all threads associated with the test case + * have terminated. + */ + bool success; /* Read only after test_case finishes! */ +}; + +void kunit_init_test(struct kunit *test, const char *name); + +int kunit_run_tests(struct kunit_suite *suite); + +/** + * kunit_test_suite() - used to register a &struct kunit_suite with KUnit. + * @suite: a statically allocated &struct kunit_suite. + * + * Registers @suite with the test framework. See &struct kunit_suite for more + * information. + * + * NOTE: Currently KUnit tests are all run as late_initcalls; this means that + * they cannot test anything where tests must run at a different init phase. One + * significant restriction resulting from this is that KUnit cannot reliably + * test anything that is initialize in the late_init phase; another is that + * KUnit is useless to test things that need to be run in an earlier init phase. + */ +#define kunit_test_suite(suite) \ + /* + * TODO(brendanhiggins@google.com): Don't run all KUnit tests as + * late_initcalls. I have some future work planned to dispatch + * all KUnit tests from the same place, and at the very least to + * do so after everything else is definitely initialized. + */ \ + static int kunit_suite_init##suite(void) \ + { \ + return kunit_run_tests(&suite); \ + } \ + late_initcall(kunit_suite_init##suite) + +void __printf(3, 4) kunit_printk(const char *level, + const struct kunit *test, + const char *fmt, ...); + +/** + * kunit_info() - Prints an INFO level message associated with the current test. + * @test: The test context object. + * @fmt: A printk() style format string. + * + * Prints an info level message associated with the test suite being run. Takes + * a variable number of format parameters just like printk(). + */ +#define kunit_info(test, fmt, ...) \ + kunit_printk(KERN_INFO, test, fmt, ##__VA_ARGS__) + +/** + * kunit_warn() - Prints a WARN level message associated with the current test. + * @test: The test context object. + * @fmt: A printk() style format string. + * + * Prints a warning level message. + */ +#define kunit_warn(test, fmt, ...) \ + kunit_printk(KERN_WARNING, test, fmt, ##__VA_ARGS__) + +/** + * kunit_err() - Prints an ERROR level message associated with the current test. + * @test: The test context object. + * @fmt: A printk() style format string. + * + * Prints an error level message. + */ +#define kunit_err(test, fmt, ...) \ + kunit_printk(KERN_ERR, test, fmt, ##__VA_ARGS__) + +#endif /* _KUNIT_TEST_H */ diff --git a/kunit/Kconfig b/kunit/Kconfig new file mode 100644 index 0000000000000..330ae83527c23 --- /dev/null +++ b/kunit/Kconfig @@ -0,0 +1,17 @@ +# +# KUnit base configuration +# + +menu "KUnit support" + +config KUNIT + bool "Enable support for unit tests (KUnit)" + help + Enables support for kernel unit tests (KUnit), a lightweight unit + testing and mocking framework for the Linux kernel. These tests are + able to be run locally on a developer's workstation without a VM or + special hardware when using UML. Can also be used on most other + architectures. For more information, please see + Documentation/dev-tools/kunit/. + +endmenu diff --git a/kunit/Makefile b/kunit/Makefile new file mode 100644 index 0000000000000..5efdc4dea2c08 --- /dev/null +++ b/kunit/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_KUNIT) += test.o diff --git a/kunit/test.c b/kunit/test.c new file mode 100644 index 0000000000000..d3dda359f99b1 --- /dev/null +++ b/kunit/test.c @@ -0,0 +1,191 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Base unit test (KUnit) API. + * + * Copyright (C) 2019, Google LLC. + * Author: Brendan Higgins <brendanhiggins@google.com> + */ + +#include <kunit/test.h> +#include <linux/kernel.h> + +static void kunit_set_failure(struct kunit *test) +{ + WRITE_ONCE(test->success, false); +} + +static int kunit_vprintk_emit(int level, const char *fmt, va_list args) +{ + return vprintk_emit(0, level, NULL, 0, fmt, args); +} + +static int kunit_printk_emit(int level, const char *fmt, ...) +{ + va_list args; + int ret; + + va_start(args, fmt); + ret = kunit_vprintk_emit(level, fmt, args); + va_end(args); + + return ret; +} + +static void kunit_vprintk(const struct kunit *test, + const char *level, + struct va_format *vaf) +{ + kunit_printk_emit(level[1] - '0', "\t# %s: %pV", test->name, vaf); +} + +static void kunit_print_tap_version(void) +{ + static bool kunit_has_printed_tap_version; + + if (!kunit_has_printed_tap_version) { + kunit_printk_emit(LOGLEVEL_INFO, "TAP version 14\n"); + kunit_has_printed_tap_version = true; + } +} + +static size_t kunit_test_cases_len(struct kunit_case *test_cases) +{ + struct kunit_case *test_case; + size_t len = 0; + + for (test_case = test_cases; test_case->run_case; test_case++) + len++; + + return len; +} + +static void kunit_print_subtest_start(struct kunit_suite *suite) +{ + kunit_print_tap_version(); + kunit_printk_emit(LOGLEVEL_INFO, "\t# Subtest: %s\n", suite->name); + kunit_printk_emit(LOGLEVEL_INFO, + "\t1..%zd\n", + kunit_test_cases_len(suite->test_cases)); +} + +static void kunit_print_ok_not_ok(bool should_indent, + bool is_ok, + size_t test_number, + const char *description) +{ + const char *indent, *ok_not_ok; + + if (should_indent) + indent = "\t"; + else + indent = ""; + + if (is_ok) + ok_not_ok = "ok"; + else + ok_not_ok = "not ok"; + + kunit_printk_emit(LOGLEVEL_INFO, + "%s%s %zd - %s\n", + indent, ok_not_ok, test_number, description); +} + +static bool kunit_suite_has_succeeded(struct kunit_suite *suite) +{ + const struct kunit_case *test_case; + + for (test_case = suite->test_cases; test_case->run_case; test_case++) + if (!test_case->success) + return false; + + return true; +} + +static void kunit_print_subtest_end(struct kunit_suite *suite) +{ + static size_t kunit_suite_counter = 1; + + kunit_print_ok_not_ok(false, + kunit_suite_has_succeeded(suite), + kunit_suite_counter++, + suite->name); +} + +static void kunit_print_test_case_ok_not_ok(struct kunit_case *test_case, + size_t test_number) +{ + kunit_print_ok_not_ok(true, + test_case->success, + test_number, + test_case->name); +} + +void kunit_init_test(struct kunit *test, const char *name) +{ + test->name = name; + test->success = true; +} + +/* + * Performs all logic to run a test case. + */ +static void kunit_run_case(struct kunit_suite *suite, + struct kunit_case *test_case) +{ + struct kunit test; + + kunit_init_test(&test, test_case->name); + + if (suite->init) { + int ret; + + ret = suite->init(&test); + if (ret) { + kunit_err(&test, "failed to initialize: %d\n", ret); + kunit_set_failure(&test); + test_case->success = test.success; + return; + } + } + + test_case->run_case(&test); + + if (suite->exit) + suite->exit(&test); + + test_case->success = test.success; +} + +int kunit_run_tests(struct kunit_suite *suite) +{ + struct kunit_case *test_case; + size_t test_case_count = 1; + + kunit_print_subtest_start(suite); + + for (test_case = suite->test_cases; test_case->run_case; test_case++) { + kunit_run_case(suite, test_case); + kunit_print_test_case_ok_not_ok(test_case, test_case_count++); + } + + kunit_print_subtest_end(suite); + + return 0; +} + +void kunit_printk(const char *level, + const struct kunit *test, + const char *fmt, ...) +{ + struct va_format vaf; + va_list args; + + va_start(args, fmt); + + vaf.fmt = fmt; + vaf.va = &args; + + kunit_vprintk(test, level, &vaf); + + va_end(args); +}