ACK: [RFC, PATCH 3/7 v2] fwts: Allow tests to be conditional on available features

[Colin Ian King]

On 28/04/14 02:35, Jeremy Kerr wrote:
> Add a .fw_features member to struct fwts_framework_test, and update the
> registration macros to allow tests to only be run if specified features
> are present.
> 
> Signed-off-by: Jeremy Kerr <jk at ozlabs.org>
> 
> ---
>  src/lib/include/fwts_framework.h |   28 ++++++++++++++++------------
>  src/lib/src/fwts_framework.c     |   14 +++++++++++++-
>  2 files changed, 29 insertions(+), 13 deletions(-)
> 
> diff --git a/src/lib/include/fwts_framework.h b/src/lib/include/fwts_framework.h
> index 713371b..142352d 100644
> --- a/src/lib/include/fwts_framework.h
> +++ b/src/lib/include/fwts_framework.h
> @@ -173,12 +173,14 @@ typedef struct fwts_framework_test {
>  	fwts_framework_ops *ops;
>  	fwts_priority priority;
>  	fwts_framework_flags flags;
> +	int fw_features;
>  	fwts_results results;			/* Per test results */
>  	bool	    was_run;
> +
>  } fwts_framework_test;
>  
>  int  fwts_framework_args(const int argc, char **argv);
> -void fwts_framework_test_add(const char *name, fwts_framework_ops *ops, const fwts_priority priority, const fwts_framework_flags flags);
> +void fwts_framework_test_add(const char *name, fwts_framework_ops *ops, const fwts_priority priority, const fwts_framework_flags flags, int fw_features);
>  int  fwts_framework_compare_test_name(void *, void *);
>  void fwts_framework_show_version(FILE *fp, const char *name);
>  
> @@ -253,16 +255,18 @@ static inline int fwts_tests_passed(const fwts_framework *fw)
>  #define FWTS_ASSERT(e, m) 	\
>  enum { FWTS_CONCAT_EXPAND(FWTS_ASSERT_ ## m ## _in_line_, __LINE__) = 1 / !!(e) }
>  
> -#define FWTS_REGISTER(name, ops, priority, flags)		\
> -/* Ensure name is not too long */				\
> -FWTS_ASSERT(FWTS_ARRAY_LEN(name) < 16,				\
> -	fwts_register_name_too_long);				\
> -								\
> -static void __test_init (void) __attribute__ ((constructor));	\
> -								\
> -static void __test_init (void)					\
> -{								\
> -	fwts_framework_test_add(name, ops, priority, flags);	\
> +#define FWTS_REGISTER_FEATURES(name, ops, priority, flags, features)	\
> +/* Ensure name is not too long */					\
> +FWTS_ASSERT(FWTS_ARRAY_LEN(name) < 16,					\
> +	fwts_register_name_too_long);					\
> +									\
> +static void __test_init (void) __attribute__ ((constructor));		\
> +									\
> +static void __test_init (void)						\
> +{									\
> +	fwts_framework_test_add(name, ops, priority, flags, features);	\
>  }
> -							
> +
> +#define FWTS_REGISTER(name, ops, priority, flags) \
> +	FWTS_REGISTER_FEATURES(name, ops, priority, flags, 0)
>  #endif
> diff --git a/src/lib/src/fwts_framework.c b/src/lib/src/fwts_framework.c
> index 39ac802..d200d26 100644
> --- a/src/lib/src/fwts_framework.c
> +++ b/src/lib/src/fwts_framework.c
> @@ -116,7 +116,8 @@ void fwts_framework_test_add(
>  	const char *name,
>  	fwts_framework_ops *ops,
>  	const fwts_priority priority,
> -	const fwts_framework_flags flags)
> +	const fwts_framework_flags flags,
> +	int fw_features)
>  {
>  	fwts_framework_test *new_test;
>  
> @@ -142,6 +143,7 @@ void fwts_framework_test_add(
>  	new_test->ops  = ops;
>  	new_test->priority = priority;
>  	new_test->flags = flags;
> +	new_test->fw_features = fw_features;
>  
>  	/* Add test, sorted on run order priority */
>  	fwts_list_add_ordered(&fwts_framework_test_list, new_test, fwts_framework_compare_priority);
> @@ -503,6 +505,16 @@ static int fwts_framework_run_test(fwts_framework *fw, fwts_framework_test *test
>  
>  	fwts_framework_minor_test_progress(fw, 0, "");
>  
> +	if (!fwts_firmware_has_features(test->fw_features)) {
> +		int missing = test->fw_features & ~fwts_firmware_features();
> +		fwts_log_info(fw, "Test skipped, missing features 0x%08x",
> +				missing);
> +		fw->current_major_test->results.skipped +=
> +			test->ops->total_tests;
> +		fw->total.skipped += test->ops->total_tests;
> +		goto done;
> +	}
> +
>  	if ((test->flags & FWTS_FLAG_ROOT_PRIV) &&
>  	    (fwts_check_root_euid(fw, true) != FWTS_OK)) {
>  		fwts_log_error(fw, "Aborted test, insufficient privilege.");
> 
FWTS_REGISTER_FEATURES() is a good idea to move this forward w/o having
to modify many of the existing tests.

Acked-by: Colin Ian King <colin.king at canonical.com>