diff mbox series

[v2,2/6] kunit: Add macro to conditionally expose declarations to tests

Message ID 20240826222015.1484-3-michal.wajdeczko@intel.com (mailing list archive)
State New
Delegated to: Brendan Higgins
Headers show
Series kunit: Add macros to help write more complex tests | expand

Commit Message

Michal Wajdeczko Aug. 26, 2024, 10:20 p.m. UTC 
The DECLARE_IF_KUNIT macro will introduces identifiers only if
CONFIG_KUNIT is enabled. Otherwise if CONFIG_KUNIT is not enabled
no identifiers from the param list will be defined.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Reviewed-by: Rae Moar <rmoar@google.com>
Reviewed-by: David Gow <davidgow@google.com>
---
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
---
 include/kunit/visibility.h | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Lucas De Marchi Aug. 27, 2024, 1:45 p.m. UTC | #1 
On Tue, Aug 27, 2024 at 12:20:11AM GMT, Michal Wajdeczko wrote:
>The DECLARE_IF_KUNIT macro will introduces identifiers only if
>CONFIG_KUNIT is enabled. Otherwise if CONFIG_KUNIT is not enabled
>no identifiers from the param list will be defined.
>
>Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>Reviewed-by: Rae Moar <rmoar@google.com>
>Reviewed-by: David Gow <davidgow@google.com>
>---
>Cc: Lucas De Marchi <lucas.demarchi@intel.com>

up to kunit maintainers, but it doesn't seem the word "DECLARE" is
entirely correct. What it's doing is expanding arg, and it doesn't
matter if it's an expression, definition, declaration.

Looking at patch 3, I think it would be more obvious to the caller if we
have:

IF_KUNIT_ELSE_EMPTY()
IF_KUNIT_ELSE_ZERO()

>---
> include/kunit/visibility.h | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
>diff --git a/include/kunit/visibility.h b/include/kunit/visibility.h
>index 0dfe35feeec6..1c23773f826c 100644
>--- a/include/kunit/visibility.h
>+++ b/include/kunit/visibility.h
>@@ -11,6 +11,13 @@
> #define _KUNIT_VISIBILITY_H
>
> #if IS_ENABLED(CONFIG_KUNIT)
>+    /**
>+     * DECLARE_IF_KUNIT - A macro that introduces identifiers only if
>+     * CONFIG_KUNIT is enabled. Otherwise if CONFIG_KUNIT is not enabled
>+     * no identifiers will be defined.
>+     * @body: identifiers to be introduced conditionally

missing an example here with fields inside a struct

Lucas De Marchi

>+     */
>+    #define DECLARE_IF_KUNIT(body...)	body
>     /**
>      * VISIBLE_IF_KUNIT - A macro that sets symbols to be static if
>      * CONFIG_KUNIT is not enabled. Otherwise if CONFIG_KUNIT is enabled
>@@ -26,6 +33,7 @@
>     #define EXPORT_SYMBOL_IF_KUNIT(symbol) EXPORT_SYMBOL_NS(symbol, \
> 	    EXPORTED_FOR_KUNIT_TESTING)
> #else
>+    #define DECLARE_IF_KUNIT(body...)
>     #define VISIBLE_IF_KUNIT static
>     #define EXPORT_SYMBOL_IF_KUNIT(symbol)
> #endif
>-- 
>2.43.0
>
Michal Wajdeczko Aug. 27, 2024, 7:56 p.m. UTC | #2 
On 27.08.2024 15:45, Lucas De Marchi wrote:
> On Tue, Aug 27, 2024 at 12:20:11AM GMT, Michal Wajdeczko wrote:
>> The DECLARE_IF_KUNIT macro will introduces identifiers only if
>> CONFIG_KUNIT is enabled. Otherwise if CONFIG_KUNIT is not enabled
>> no identifiers from the param list will be defined.
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Reviewed-by: Rae Moar <rmoar@google.com>
>> Reviewed-by: David Gow <davidgow@google.com>
>> ---
>> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> 
> up to kunit maintainers, but it doesn't seem the word "DECLARE" is
> entirely correct. What it's doing is expanding arg, and it doesn't
> matter if it's an expression, definition, declaration.

hmm, while this is true for statement & declarations (as both have
similar notation) but not sure about the expression (that's why we have
patch 3)

> 
> Looking at patch 3, I think it would be more obvious to the caller if we
> have:
> 
> IF_KUNIT_ELSE_EMPTY()
> IF_KUNIT_ELSE_ZERO()

existing macros in this file are named as xxx_IF_KUNIT so maybe we
should try to follow that pattern...

so maybe (like BUILD_BUG_ON_ZERO)

	ONLY_IF_KUNIT(body...)
	ONLY_IF_KUNIT_ZERO(expr...)

> 
>> ---
>> include/kunit/visibility.h | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/include/kunit/visibility.h b/include/kunit/visibility.h
>> index 0dfe35feeec6..1c23773f826c 100644
>> --- a/include/kunit/visibility.h
>> +++ b/include/kunit/visibility.h
>> @@ -11,6 +11,13 @@
>> #define _KUNIT_VISIBILITY_H
>>
>> #if IS_ENABLED(CONFIG_KUNIT)
>> +    /**
>> +     * DECLARE_IF_KUNIT - A macro that introduces identifiers only if
>> +     * CONFIG_KUNIT is enabled. Otherwise if CONFIG_KUNIT is not enabled
>> +     * no identifiers will be defined.
>> +     * @body: identifiers to be introduced conditionally
> 
> missing an example here with fields inside a struct

would this work?

Example:

	struct example {
		int foo;
		/* private: test only */
		DECLARE_IF_KUNIT(int bar);
	};

> 
> Lucas De Marchi
> 
>> +     */
>> +    #define DECLARE_IF_KUNIT(body...)    body
>>     /**
>>      * VISIBLE_IF_KUNIT - A macro that sets symbols to be static if
>>      * CONFIG_KUNIT is not enabled. Otherwise if CONFIG_KUNIT is enabled
>> @@ -26,6 +33,7 @@
>>     #define EXPORT_SYMBOL_IF_KUNIT(symbol) EXPORT_SYMBOL_NS(symbol, \
>>         EXPORTED_FOR_KUNIT_TESTING)
>> #else
>> +    #define DECLARE_IF_KUNIT(body...)
>>     #define VISIBLE_IF_KUNIT static
>>     #define EXPORT_SYMBOL_IF_KUNIT(symbol)
>> #endif
>> -- 
>> 2.43.0
>>
diff mbox series

Patch

diff --git a/include/kunit/visibility.h b/include/kunit/visibility.h
index 0dfe35feeec6..1c23773f826c 100644
--- a/include/kunit/visibility.h
+++ b/include/kunit/visibility.h
@@ -11,6 +11,13 @@ 
 #define _KUNIT_VISIBILITY_H
 
 #if IS_ENABLED(CONFIG_KUNIT)
+    /**
+     * DECLARE_IF_KUNIT - A macro that introduces identifiers only if
+     * CONFIG_KUNIT is enabled. Otherwise if CONFIG_KUNIT is not enabled
+     * no identifiers will be defined.
+     * @body: identifiers to be introduced conditionally
+     */
+    #define DECLARE_IF_KUNIT(body...)	body
     /**
      * VISIBLE_IF_KUNIT - A macro that sets symbols to be static if
      * CONFIG_KUNIT is not enabled. Otherwise if CONFIG_KUNIT is enabled
@@ -26,6 +33,7 @@ 
     #define EXPORT_SYMBOL_IF_KUNIT(symbol) EXPORT_SYMBOL_NS(symbol, \
 	    EXPORTED_FOR_KUNIT_TESTING)
 #else
+    #define DECLARE_IF_KUNIT(body...)
     #define VISIBLE_IF_KUNIT static
     #define EXPORT_SYMBOL_IF_KUNIT(symbol)
 #endif