[1/2] fwts: framework: Add --log-level option

From: Colin Ian King <colin.king@canonical.com>

Allow users to specify a log error level threshold, option is:

--log-level=[critical|high|medium|low|info|all]

where the level specifies the log level and above to log. For example,
'critical' just logs failures of level critical,  'medium' logs failures
of medium, high and critical, 'all' log all failures.  Note that the
'all' level is identical to 'info'.  The change includes:

1) Changes to the logging to pass the entire fwts_framework rather than
   the log to be able to filter out at the lowest log level.
2) Filtering at the error accounting level, so we only account for
   errors at the given log level or above
3) Filtering at the summary information, again, so we only account for
   errord at the given log level or above

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 doc/fwts.1                       |  7 ++++++
 src/lib/include/fwts_framework.h |  9 ++++++-
 src/lib/include/fwts_log.h       | 31 ++++++++++++-----------
 src/lib/src/fwts_framework.c     | 54 ++++++++++++++++++++++++++++++++++++----
 src/lib/src/fwts_log.c           |  9 +++++--
 src/lib/src/fwts_summary.c       | 16 ++++++++++--
 6 files changed, 101 insertions(+), 25 deletions(-)

On 2015-12-10 08:20 PM, Colin King wrote:
> From: Colin Ian King<colin.king@canonical.com>
>
> Allow users to specify a log error level threshold, option is:
>
> --log-level=[critical|high|medium|low|info|all]
>
> where the level specifies the log level and above to log. For example,
> 'critical' just logs failures of level critical,  'medium' logs failures
> of medium, high and critical, 'all' log all failures.  Note that the
> 'all' level is identical to 'info'.  The change includes:
>
> 1) Changes to the logging to pass the entire fwts_framework rather than
>     the log to be able to filter out at the lowest log level.
> 2) Filtering at the error accounting level, so we only account for
>     errors at the given log level or above
> 3) Filtering at the summary information, again, so we only account for
>     errord at the given log level or above
>
> Signed-off-by: Colin Ian King<colin.king@canonical.com>
> ---
>   doc/fwts.1                       |  7 ++++++
>   src/lib/include/fwts_framework.h |  9 ++++++-
>   src/lib/include/fwts_log.h       | 31 ++++++++++++-----------
>   src/lib/src/fwts_framework.c     | 54 ++++++++++++++++++++++++++++++++++++----
>   src/lib/src/fwts_log.c           |  9 +++++--
>   src/lib/src/fwts_summary.c       | 16 ++++++++++--
>   6 files changed, 101 insertions(+), 25 deletions(-)
>
> diff --git a/doc/fwts.1 b/doc/fwts.1
> index edefc0b..e299b26 100644
> --- a/doc/fwts.1
> +++ b/doc/fwts.1
> @@ -177,6 +177,13 @@ specify the information in each log line. The following specifiers are available
>   .br
>   e.g. \-\-log\-format="%date %time [%field] (%owner): "
>   .TP
> +.B \-\-log\-level [critical|high|medium|low|info|all]

Did you miss a "=" here? for example, it should be
+.B \-\-log\-level=[critical|high|medium|low|info|all]

On 15/12/15 02:10, Alex Hung wrote:
> On 2015-12-10 08:20 PM, Colin King wrote:
>> From: Colin Ian King<colin.king@canonical.com>
>>
>> Allow users to specify a log error level threshold, option is:
>>
>> --log-level=[critical|high|medium|low|info|all]
>>
>> where the level specifies the log level and above to log. For example,
>> 'critical' just logs failures of level critical,  'medium' logs failures
>> of medium, high and critical, 'all' log all failures.  Note that the
>> 'all' level is identical to 'info'.  The change includes:
>>
>> 1) Changes to the logging to pass the entire fwts_framework rather than
>>     the log to be able to filter out at the lowest log level.
>> 2) Filtering at the error accounting level, so we only account for
>>     errors at the given log level or above
>> 3) Filtering at the summary information, again, so we only account for
>>     errord at the given log level or above
>>
>> Signed-off-by: Colin Ian King<colin.king@canonical.com>
>> ---
>>   doc/fwts.1                       |  7 ++++++
>>   src/lib/include/fwts_framework.h |  9 ++++++-
>>   src/lib/include/fwts_log.h       | 31 ++++++++++++-----------
>>   src/lib/src/fwts_framework.c     | 54
>> ++++++++++++++++++++++++++++++++++++----
>>   src/lib/src/fwts_log.c           |  9 +++++--
>>   src/lib/src/fwts_summary.c       | 16 ++++++++++--
>>   6 files changed, 101 insertions(+), 25 deletions(-)
>>
>> diff --git a/doc/fwts.1 b/doc/fwts.1
>> index edefc0b..e299b26 100644
>> --- a/doc/fwts.1
>> +++ b/doc/fwts.1
>> @@ -177,6 +177,13 @@ specify the information in each log line. The
>> following specifiers are available
>>   .br
>>   e.g. \-\-log\-format="%date %time [%field] (%owner): "
>>   .TP
>> +.B \-\-log\-level [critical|high|medium|low|info|all]
> 
> Did you miss a "=" here? for example, it should be
> +.B \-\-log\-level=[critical|high|medium|low|info|all]
> 
> 
The code uses the gnu opt parser, so the '=' is optional and not mandatory.

Colin

On 2015-12-10 08:20 PM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Allow users to specify a log error level threshold, option is:
>
> --log-level=[critical|high|medium|low|info|all]
>
> where the level specifies the log level and above to log. For example,
> 'critical' just logs failures of level critical,  'medium' logs failures
> of medium, high and critical, 'all' log all failures.  Note that the
> 'all' level is identical to 'info'.  The change includes:
>
> 1) Changes to the logging to pass the entire fwts_framework rather than
>     the log to be able to filter out at the lowest log level.
> 2) Filtering at the error accounting level, so we only account for
>     errors at the given log level or above
> 3) Filtering at the summary information, again, so we only account for
>     errord at the given log level or above
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>   doc/fwts.1                       |  7 ++++++
>   src/lib/include/fwts_framework.h |  9 ++++++-
>   src/lib/include/fwts_log.h       | 31 ++++++++++++-----------
>   src/lib/src/fwts_framework.c     | 54 ++++++++++++++++++++++++++++++++++++----
>   src/lib/src/fwts_log.c           |  9 +++++--
>   src/lib/src/fwts_summary.c       | 16 ++++++++++--
>   6 files changed, 101 insertions(+), 25 deletions(-)
>
> diff --git a/doc/fwts.1 b/doc/fwts.1
> index edefc0b..e299b26 100644
> --- a/doc/fwts.1
> +++ b/doc/fwts.1
> @@ -177,6 +177,13 @@ specify the information in each log line. The following specifiers are available
>   .br
>   e.g. \-\-log\-format="%date %time [%field] (%owner): "
>   .TP
> +.B \-\-log\-level [critical|high|medium|low|info|all]
> +specify the test failure level to log. Test failure levels equal to and
> +higher than the specified are logged and recorded as failures. The default
> +is 'all' (which is identical to 'info').  For example, a log level of 'medium'
> +will just log test failures of level 'medium', 'high' and 'critical',
> +where as a log level of 'critical' will just log 'critical' level failures.
> +.TP
>   .B \-\-log\-type
>   specify the log type. Currently plaintext, json and xml log types are available and the
>   default is plaintext.
> diff --git a/src/lib/include/fwts_framework.h b/src/lib/include/fwts_framework.h
> index 0ae8321..fcd65b7 100644
> --- a/src/lib/include/fwts_framework.h
> +++ b/src/lib/include/fwts_framework.h
> @@ -24,6 +24,8 @@
>   #include <string.h>
>   #include <stdbool.h>
>
> +typedef struct fwts_framework fwts_framework;
> +
>   #include "fwts_log.h"
>   #include "fwts_list.h"
>   #include "fwts_acpica_mode.h"
> @@ -104,7 +106,7 @@ static inline void fwts_framework_summate_results(fwts_results *total, fwts_resu
>   /*
>    *  Test framework context
>    */
> -typedef struct {
> +typedef struct fwts_framework {
>   	uint32_t magic;				/* identify struct magic */
>   	fwts_log *results;			/* log for test results */
>   	char *results_logname;			/* filename of results log */
> @@ -133,6 +135,7 @@ typedef struct {
>   	int minor_test_progress;		/* Percentage completion of current test */
>   	bool print_summary;			/* Print summary of results at end of test runs */
>   	fwts_log_level failed_level;		/* Bit mask of failed levels in test run */
> +	fwts_log_level filter_level;		/* --log-level option filter */
>
>   	int firmware_type;			/* Type of firmware */
>   	bool show_progress;			/* Show progress while running current test */
> @@ -274,4 +277,8 @@ static void __test_init (void)						\
>
>   #define FWTS_REGISTER(name, ops, priority, flags) \
>   	FWTS_REGISTER_FEATURES(name, ops, priority, flags, 0)
> +
> +#define FWTS_LEVEL_IGNORE(fw, level)	\
> +	((level) && !((level) & fw->filter_level))
> +
>   #endif
> diff --git a/src/lib/include/fwts_log.h b/src/lib/include/fwts_log.h
> index 4772769..7514a53 100644
> --- a/src/lib/include/fwts_log.h
> +++ b/src/lib/include/fwts_log.h
> @@ -23,6 +23,7 @@
>   #include <stdio.h>
>   #include <stdarg.h>
>
> +#include "fwts_framework.h"
>   #include "fwts_list.h"
>
>   #define LOG_MAGIC		(0xfe23ab13)
> @@ -57,7 +58,8 @@ typedef enum {
>   	LOG_LEVEL_HIGH      = 0x00000002,
>   	LOG_LEVEL_MEDIUM    = 0x00000004,
>   	LOG_LEVEL_LOW       = 0x00000008,
> -	LOG_LEVEL_INFO      = 0x00000010
> +	LOG_LEVEL_INFO      = 0x00000010,
> +	LOG_LEVEL_ALL       = 0x0000001f
>   } fwts_log_level;
>
>   /*
> @@ -122,9 +124,8 @@ extern const char *fwts_log_format;
>
>   fwts_log *fwts_log_open(const char* owner, const char *name, const char *mode, const fwts_log_type);
>   int       fwts_log_close(fwts_log *log);
> -int       fwts_log_printf(fwts_log *log, const fwts_log_field field, const fwts_log_level level, const char *status, const char *label, const char *prefix, const char *fmt, ...)
> +int       fwts_log_printf(const fwts_framework *fw, const fwts_log_field field, const fwts_log_level level, const char *status, const char *label, const char *prefix, const char *fmt, ...)
>   	__attribute__((format(printf, 7, 8)));
> -int       fwts_log_vprintf(fwts_log *log, const fwts_log_field field, const fwts_log_level level, const char *status, const char *label, const char *prefix, const char *fmt, va_list args);
>   void      fwts_log_newline(fwts_log *log);
>   void      fwts_log_underline(fwts_log *log, const int ch);
>   void      fwts_log_set_field_filter(const char *str);
> @@ -151,39 +152,39 @@ static inline int fwts_log_type_count(fwts_log_type type)
>   }
>
>   #define fwts_log_result(fw, fmt, args...)	\
> -	fwts_log_printf(fw->results, LOG_RESULT, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
> +	fwts_log_printf(fw, LOG_RESULT, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
>
>   #define fwts_log_warning(fw, fmt, args...)	\
> -	fwts_log_printf(fw->results, LOG_WARNING, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
> +	fwts_log_printf(fw, LOG_WARNING, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
>
>   #define fwts_log_warning_verbatum(fw, fmt, args...)	\
> -	fwts_log_printf(fw->results, LOG_WARNING | LOG_VERBATUM, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
> +	fwts_log_printf(fw, LOG_WARNING | LOG_VERBATUM, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
>
>   #define fwts_log_error(fw, fmt, args...)	\
> -	fwts_log_printf(fw->results, LOG_ERROR, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
> +	fwts_log_printf(fw, LOG_ERROR, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
>
>   #define fwts_log_error_verbatum(fw, fmt, args...)	\
> -	fwts_log_printf(fw->results, LOG_ERROR | LOG_VERBATUM, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
> +	fwts_log_printf(fw, LOG_ERROR | LOG_VERBATUM, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
>
>   #define fwts_log_info(fw, fmt, args...)	\
> -	fwts_log_printf(fw->results, LOG_INFO, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
> +	fwts_log_printf(fw, LOG_INFO, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
>
>   #define fwts_log_info_verbatum(fw, fmt, args...)	\
> -	fwts_log_printf(fw->results, LOG_INFO | LOG_VERBATUM, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
> +	fwts_log_printf(fw, LOG_INFO | LOG_VERBATUM, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
>
>   #define fwts_log_summary(fw, fmt, args...)	\
> -	fwts_log_printf(fw->results, LOG_SUMMARY, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
> +	fwts_log_printf(fw, LOG_SUMMARY, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
>
>   #define fwts_log_summary_verbatum(fw, fmt, args...)	\
> -	fwts_log_printf(fw->results, LOG_SUMMARY | LOG_VERBATUM, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
> +	fwts_log_printf(fw, LOG_SUMMARY | LOG_VERBATUM, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
>
>   #define fwts_log_advice(fw, fmt, args...)	\
> -	fwts_log_printf(fw->results, LOG_ADVICE, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
> +	fwts_log_printf(fw, LOG_ADVICE, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
>
>   #define fwts_log_heading(fw, fmt, args...)	\
> -	fwts_log_printf(fw->results, LOG_HEADING, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
> +	fwts_log_printf(fw, LOG_HEADING, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
>
>   #define fwts_log_nl(fw) \
> -	fwts_log_printf(fw->results, LOG_NEWLINE, LOG_LEVEL_NONE, "", "", "", "%s", "")
> +	fwts_log_printf(fw, LOG_NEWLINE, LOG_LEVEL_NONE, "", "", "", "%s", "")
>
>   #endif
> diff --git a/src/lib/src/fwts_framework.c b/src/lib/src/fwts_framework.c
> index 1d44263..808e211 100644
> --- a/src/lib/src/fwts_framework.c
> +++ b/src/lib/src/fwts_framework.c
> @@ -36,6 +36,11 @@ typedef struct {
>   	fwts_framework_flags flag;	/* Mask of category */
>   } fwts_categories;
>
> +typedef struct {
> +	const char *name;
> +	const fwts_log_level filter_level;
> +} fwts_log_levels;
> +
>   /* Suffix ".log", ".xml", etc gets automatically appended */
>   #define RESULTS_LOG	"results"
>
> @@ -66,6 +71,16 @@ static fwts_categories categories[] = {
>   	{ NULL,				0 },
>   };
>
> +static fwts_log_levels log_levels[] = {
> +	{ "critical",			LOG_LEVEL_CRITICAL },
> +	{ "high",			LOG_LEVEL_CRITICAL | LOG_LEVEL_HIGH },
> +	{ "medium",			LOG_LEVEL_CRITICAL | LOG_LEVEL_HIGH | LOG_LEVEL_MEDIUM },
> +	{ "low",			LOG_LEVEL_CRITICAL | LOG_LEVEL_HIGH | LOG_LEVEL_MEDIUM | LOG_LEVEL_LOW },
> +	{ "info",			LOG_LEVEL_CRITICAL | LOG_LEVEL_HIGH | LOG_LEVEL_MEDIUM | LOG_LEVEL_LOW | LOG_LEVEL_INFO },
> +	{ "all",			LOG_LEVEL_ALL },
> +	{ NULL,				0 },
> +};
> +
>   static fwts_list tests_to_skip;
>
>   static fwts_option fwts_framework_options[] = {
> @@ -111,6 +126,7 @@ static fwts_option fwts_framework_options[] = {
>   	{ "show-tests-categories","", 0, "Show tests and associated categories." },
>   	{ "acpitests",		"",   0, "Run general ACPI tests." },
>   	{ "acpicompliance",	"",   0, "Run ACPI tests for spec compliance." },
> +	{ "log-level",		"",   1, "Specify error level to report failed test messages," },
>   	{ NULL, NULL, 0, NULL }
>   };
>
> @@ -768,7 +784,8 @@ void fwts_error_inc(fwts_framework *fw, const char *label, int *count)
>    *  fwts_framework_log()
>    *	log a test result
>    */
> -void fwts_framework_log(fwts_framework *fw,
> +void fwts_framework_log(
> +	fwts_framework *fw,
>   	fwts_log_field field,
>   	const char *label,
>   	fwts_log_level level,
> @@ -778,7 +795,7 @@ void fwts_framework_log(fwts_framework *fw,
>   	char buffer[4096];
>   	char prefix[256];
>   	char *str = fwts_log_field_to_str_upper(field);
> -	bool do_count = true;
> +	bool do_count = !FWTS_LEVEL_IGNORE(fw, level);
>
>   	if (fmt) {
>   		va_list ap;
> @@ -797,7 +814,7 @@ void fwts_framework_log(fwts_framework *fw,
>   		} else {
>   			fwts_log_nl(fw);
>   			snprintf(prefix, sizeof(prefix), "%s: ", str);
> -			fwts_log_printf(fw->results, field, level, str, label, prefix, "%s", buffer);
> +			fwts_log_printf(fw, field, level, str, label, prefix, "%s", buffer);
>   			fwts_log_nl(fw);
>   		}
>   		break;
> @@ -812,7 +829,7 @@ void fwts_framework_log(fwts_framework *fw,
>   			fwts_summary_add(fw, fw->current_major_test->name, level, buffer);
>   			snprintf(prefix, sizeof(prefix), "%s [%s] %s: Test %d, ",
>   				str, fwts_log_level_to_str(level), label, fw->current_minor_test_num);
> -			fwts_log_printf(fw->results, field, level, str, label, prefix, "%s", buffer);
> +			fwts_log_printf(fw, field, level, str, label, prefix, "%s", buffer);
>   		}
>   		break;
>   	case LOG_PASSED:
> @@ -821,7 +838,7 @@ void fwts_framework_log(fwts_framework *fw,
>   	case LOG_ABORTED:
>   		snprintf(prefix, sizeof(prefix), "%s: Test %d, ",
>   			str, fw->current_minor_test_num);
> -		fwts_log_printf(fw->results, field, level, str, label, prefix, "%s", buffer);
> +		fwts_log_printf(fw, field, level, str, label, prefix, "%s", buffer);
>   		break;
>   	case LOG_INFOONLY:
>   		break;	/* no-op */
> @@ -1088,6 +1105,28 @@ static int fwts_framework_pm_method_parse(fwts_framework *fw, const char *arg)
>   	return FWTS_OK;
>   }
>
> +/*
> + *  fwts_framework_ll_parse()
> + *	parse log level option
> + */
> +static int fwts_framework_ll_parse(fwts_framework *fw, const char *arg)
> +{
> +	int i;
> +
> +	for (i = 0; log_levels[i].name; i++) {
> +		if (!strcmp(arg, log_levels[i].name)) {
> +			fw->filter_level = log_levels[i].filter_level;
> +			return FWTS_OK;
> +		}
> +	}
> +	fprintf(stderr, "--log-level supports levels:");
> +	for (i = 0; log_levels[i].name; i++)
> +		fprintf(stderr, " %s", log_levels[i].name);
> +	fprintf(stderr, "\n");
> +
> +	return FWTS_ERROR;
> +}
> +
>   int fwts_framework_options_handler(fwts_framework *fw, int argc, char * const argv[], int option_char, int long_index)
>   {
>   	FWTS_UNUSED(argc);
> @@ -1237,6 +1276,10 @@ int fwts_framework_options_handler(fwts_framework *fw, int argc, char * const ar
>   		case 41: /* --acpicompliance */
>   			fw->flags |= FWTS_FLAG_TEST_COMPLIANCE_ACPI;
>   			break;
> +		case 42: /* --log-level */
> +			if (fwts_framework_ll_parse(fw, optarg) != FWTS_OK)
> +				return FWTS_ERROR;
> +			break;
>   		}
>   		break;
>   	case 'a': /* --all */
> @@ -1349,6 +1392,7 @@ int fwts_framework_args(const int argc, char **argv)
>   	fw->flags = FWTS_FLAG_DEFAULT |
>   		    FWTS_FLAG_SHOW_PROGRESS;
>   	fw->log_type = LOG_TYPE_PLAINTEXT;
> +	fw->filter_level = LOG_LEVEL_ALL;
>
>   	fwts_list_init(&fw->errors_filter_keep);
>   	fwts_list_init(&fw->errors_filter_discard);
> diff --git a/src/lib/src/fwts_log.c b/src/lib/src/fwts_log.c
> index fde6035..c526e6c 100644
> --- a/src/lib/src/fwts_log.c
> +++ b/src/lib/src/fwts_log.c
> @@ -393,21 +393,26 @@ static char *fwts_log_filename(const char *filename, const fwts_log_type type)
>   }
>
>   /*
> - *  fwts_log_vprintf()
> + *  fwts_log_printf()
>    *	printf to a log
>    */
> -int fwts_log_printf(fwts_log *log,
> +int fwts_log_printf(
> +	const fwts_framework *fw,
>   	const fwts_log_field field,
>   	const fwts_log_level level,
>   	const char *status,
>   	const char *label,
>   	const char *prefix,
>   	const char *fmt, ...)
> +
>   {
>   	int ret = 0;
> +	fwts_log *log = fw->results;
>
>   	if (!((field & LOG_FIELD_MASK) & fwts_log_filter))
>   		return ret;
> +	if (FWTS_LEVEL_IGNORE(fw, level))
> +		return ret;
>
>   	if (log && log->magic == LOG_MAGIC) {
>   		char buffer[LOG_MAX_BUF_SIZE];
> diff --git a/src/lib/src/fwts_summary.c b/src/lib/src/fwts_summary.c
> index d0fa905..6c2ba92 100644
> --- a/src/lib/src/fwts_summary.c
> +++ b/src/lib/src/fwts_summary.c
> @@ -46,6 +46,14 @@ static const char *summary_names[] = {
>   	"Other"
>   };
>
> +static const int summary_levels[] = {
> +	LOG_LEVEL_CRITICAL,
> +	LOG_LEVEL_HIGH,
> +	LOG_LEVEL_MEDIUM,
> +	LOG_LEVEL_LOW,
> +	LOG_LEVEL_NONE
> +};
> +
>   /* list of summary items per error level */
>   static fwts_list *fwts_summaries[SUMMARY_MAX];
>
> @@ -127,7 +135,8 @@ int fwts_summary_add(
>   	bool summary_item_found = false;
>   	int index = fwts_summary_level_to_index(level);
>
> -	FWTS_UNUSED(fw);
> +	if (FWTS_LEVEL_IGNORE(fw, level))
> +		return FWTS_OK;
>
>   	/* Does the text already exist? - search for it */
>   	fwts_list_foreach(item, fwts_summaries[index]) {
> @@ -186,7 +195,10 @@ int fwts_summary_report(fwts_framework *fw, fwts_list *test_list)
>   	fwts_log_underline(fw->results, '=');
>   	fwts_log_nl(fw);
>
> -	for (i=0;i<SUMMARY_MAX;i++) {
> +	for (i = 0; i < SUMMARY_MAX; i++) {
> +		if (FWTS_LEVEL_IGNORE(fw, summary_levels[i]))
> +			continue;
> +
>   		fwts_log_section_begin(fw->results, "failure");
>
>   		if (fwts_summaries[i]->len) {
>

Acked-by: Alex Hung <alex.hung@canonical.com>

On 2015年12月10日 20:20, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Allow users to specify a log error level threshold, option is:
>
> --log-level=[critical|high|medium|low|info|all]
>
> where the level specifies the log level and above to log. For example,
> 'critical' just logs failures of level critical,  'medium' logs failures
> of medium, high and critical, 'all' log all failures.  Note that the
> 'all' level is identical to 'info'.  The change includes:
>
> 1) Changes to the logging to pass the entire fwts_framework rather than
>     the log to be able to filter out at the lowest log level.
> 2) Filtering at the error accounting level, so we only account for
>     errors at the given log level or above
> 3) Filtering at the summary information, again, so we only account for
>     errord at the given log level or above
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>   doc/fwts.1                       |  7 ++++++
>   src/lib/include/fwts_framework.h |  9 ++++++-
>   src/lib/include/fwts_log.h       | 31 ++++++++++++-----------
>   src/lib/src/fwts_framework.c     | 54 ++++++++++++++++++++++++++++++++++++----
>   src/lib/src/fwts_log.c           |  9 +++++--
>   src/lib/src/fwts_summary.c       | 16 ++++++++++--
>   6 files changed, 101 insertions(+), 25 deletions(-)
>
> diff --git a/doc/fwts.1 b/doc/fwts.1
> index edefc0b..e299b26 100644
> --- a/doc/fwts.1
> +++ b/doc/fwts.1
> @@ -177,6 +177,13 @@ specify the information in each log line. The following specifiers are available
>   .br
>   e.g. \-\-log\-format="%date %time [%field] (%owner): "
>   .TP
> +.B \-\-log\-level [critical|high|medium|low|info|all]
> +specify the test failure level to log. Test failure levels equal to and
> +higher than the specified are logged and recorded as failures. The default
> +is 'all' (which is identical to 'info').  For example, a log level of 'medium'
> +will just log test failures of level 'medium', 'high' and 'critical',
> +where as a log level of 'critical' will just log 'critical' level failures.
> +.TP
>   .B \-\-log\-type
>   specify the log type. Currently plaintext, json and xml log types are available and the
>   default is plaintext.
> diff --git a/src/lib/include/fwts_framework.h b/src/lib/include/fwts_framework.h
> index 0ae8321..fcd65b7 100644
> --- a/src/lib/include/fwts_framework.h
> +++ b/src/lib/include/fwts_framework.h
> @@ -24,6 +24,8 @@
>   #include <string.h>
>   #include <stdbool.h>
>
> +typedef struct fwts_framework fwts_framework;
> +
>   #include "fwts_log.h"
>   #include "fwts_list.h"
>   #include "fwts_acpica_mode.h"
> @@ -104,7 +106,7 @@ static inline void fwts_framework_summate_results(fwts_results *total, fwts_resu
>   /*
>    *  Test framework context
>    */
> -typedef struct {
> +typedef struct fwts_framework {
>   	uint32_t magic;				/* identify struct magic */
>   	fwts_log *results;			/* log for test results */
>   	char *results_logname;			/* filename of results log */
> @@ -133,6 +135,7 @@ typedef struct {
>   	int minor_test_progress;		/* Percentage completion of current test */
>   	bool print_summary;			/* Print summary of results at end of test runs */
>   	fwts_log_level failed_level;		/* Bit mask of failed levels in test run */
> +	fwts_log_level filter_level;		/* --log-level option filter */
>
>   	int firmware_type;			/* Type of firmware */
>   	bool show_progress;			/* Show progress while running current test */
> @@ -274,4 +277,8 @@ static void __test_init (void)						\
>
>   #define FWTS_REGISTER(name, ops, priority, flags) \
>   	FWTS_REGISTER_FEATURES(name, ops, priority, flags, 0)
> +
> +#define FWTS_LEVEL_IGNORE(fw, level)	\
> +	((level) && !((level) & fw->filter_level))
> +
>   #endif
> diff --git a/src/lib/include/fwts_log.h b/src/lib/include/fwts_log.h
> index 4772769..7514a53 100644
> --- a/src/lib/include/fwts_log.h
> +++ b/src/lib/include/fwts_log.h
> @@ -23,6 +23,7 @@
>   #include <stdio.h>
>   #include <stdarg.h>
>
> +#include "fwts_framework.h"
>   #include "fwts_list.h"
>
>   #define LOG_MAGIC		(0xfe23ab13)
> @@ -57,7 +58,8 @@ typedef enum {
>   	LOG_LEVEL_HIGH      = 0x00000002,
>   	LOG_LEVEL_MEDIUM    = 0x00000004,
>   	LOG_LEVEL_LOW       = 0x00000008,
> -	LOG_LEVEL_INFO      = 0x00000010
> +	LOG_LEVEL_INFO      = 0x00000010,
> +	LOG_LEVEL_ALL       = 0x0000001f
>   } fwts_log_level;
>
>   /*
> @@ -122,9 +124,8 @@ extern const char *fwts_log_format;
>
>   fwts_log *fwts_log_open(const char* owner, const char *name, const char *mode, const fwts_log_type);
>   int       fwts_log_close(fwts_log *log);
> -int       fwts_log_printf(fwts_log *log, const fwts_log_field field, const fwts_log_level level, const char *status, const char *label, const char *prefix, const char *fmt, ...)
> +int       fwts_log_printf(const fwts_framework *fw, const fwts_log_field field, const fwts_log_level level, const char *status, const char *label, const char *prefix, const char *fmt, ...)
>   	__attribute__((format(printf, 7, 8)));
> -int       fwts_log_vprintf(fwts_log *log, const fwts_log_field field, const fwts_log_level level, const char *status, const char *label, const char *prefix, const char *fmt, va_list args);
>   void      fwts_log_newline(fwts_log *log);
>   void      fwts_log_underline(fwts_log *log, const int ch);
>   void      fwts_log_set_field_filter(const char *str);
> @@ -151,39 +152,39 @@ static inline int fwts_log_type_count(fwts_log_type type)
>   }
>
>   #define fwts_log_result(fw, fmt, args...)	\
> -	fwts_log_printf(fw->results, LOG_RESULT, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
> +	fwts_log_printf(fw, LOG_RESULT, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
>
>   #define fwts_log_warning(fw, fmt, args...)	\
> -	fwts_log_printf(fw->results, LOG_WARNING, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
> +	fwts_log_printf(fw, LOG_WARNING, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
>
>   #define fwts_log_warning_verbatum(fw, fmt, args...)	\
> -	fwts_log_printf(fw->results, LOG_WARNING | LOG_VERBATUM, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
> +	fwts_log_printf(fw, LOG_WARNING | LOG_VERBATUM, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
>
>   #define fwts_log_error(fw, fmt, args...)	\
> -	fwts_log_printf(fw->results, LOG_ERROR, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
> +	fwts_log_printf(fw, LOG_ERROR, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
>
>   #define fwts_log_error_verbatum(fw, fmt, args...)	\
> -	fwts_log_printf(fw->results, LOG_ERROR | LOG_VERBATUM, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
> +	fwts_log_printf(fw, LOG_ERROR | LOG_VERBATUM, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
>
>   #define fwts_log_info(fw, fmt, args...)	\
> -	fwts_log_printf(fw->results, LOG_INFO, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
> +	fwts_log_printf(fw, LOG_INFO, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
>
>   #define fwts_log_info_verbatum(fw, fmt, args...)	\
> -	fwts_log_printf(fw->results, LOG_INFO | LOG_VERBATUM, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
> +	fwts_log_printf(fw, LOG_INFO | LOG_VERBATUM, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
>
>   #define fwts_log_summary(fw, fmt, args...)	\
> -	fwts_log_printf(fw->results, LOG_SUMMARY, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
> +	fwts_log_printf(fw, LOG_SUMMARY, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
>
>   #define fwts_log_summary_verbatum(fw, fmt, args...)	\
> -	fwts_log_printf(fw->results, LOG_SUMMARY | LOG_VERBATUM, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
> +	fwts_log_printf(fw, LOG_SUMMARY | LOG_VERBATUM, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
>
>   #define fwts_log_advice(fw, fmt, args...)	\
> -	fwts_log_printf(fw->results, LOG_ADVICE, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
> +	fwts_log_printf(fw, LOG_ADVICE, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
>
>   #define fwts_log_heading(fw, fmt, args...)	\
> -	fwts_log_printf(fw->results, LOG_HEADING, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
> +	fwts_log_printf(fw, LOG_HEADING, LOG_LEVEL_NONE, "", "", "", fmt, ## args)
>
>   #define fwts_log_nl(fw) \
> -	fwts_log_printf(fw->results, LOG_NEWLINE, LOG_LEVEL_NONE, "", "", "", "%s", "")
> +	fwts_log_printf(fw, LOG_NEWLINE, LOG_LEVEL_NONE, "", "", "", "%s", "")
>
>   #endif
> diff --git a/src/lib/src/fwts_framework.c b/src/lib/src/fwts_framework.c
> index 1d44263..808e211 100644
> --- a/src/lib/src/fwts_framework.c
> +++ b/src/lib/src/fwts_framework.c
> @@ -36,6 +36,11 @@ typedef struct {
>   	fwts_framework_flags flag;	/* Mask of category */
>   } fwts_categories;
>
> +typedef struct {
> +	const char *name;
> +	const fwts_log_level filter_level;
> +} fwts_log_levels;
> +
>   /* Suffix ".log", ".xml", etc gets automatically appended */
>   #define RESULTS_LOG	"results"
>
> @@ -66,6 +71,16 @@ static fwts_categories categories[] = {
>   	{ NULL,				0 },
>   };
>
> +static fwts_log_levels log_levels[] = {
> +	{ "critical",			LOG_LEVEL_CRITICAL },
> +	{ "high",			LOG_LEVEL_CRITICAL | LOG_LEVEL_HIGH },
> +	{ "medium",			LOG_LEVEL_CRITICAL | LOG_LEVEL_HIGH | LOG_LEVEL_MEDIUM },
> +	{ "low",			LOG_LEVEL_CRITICAL | LOG_LEVEL_HIGH | LOG_LEVEL_MEDIUM | LOG_LEVEL_LOW },
> +	{ "info",			LOG_LEVEL_CRITICAL | LOG_LEVEL_HIGH | LOG_LEVEL_MEDIUM | LOG_LEVEL_LOW | LOG_LEVEL_INFO },
> +	{ "all",			LOG_LEVEL_ALL },
> +	{ NULL,				0 },
> +};
> +
>   static fwts_list tests_to_skip;
>
>   static fwts_option fwts_framework_options[] = {
> @@ -111,6 +126,7 @@ static fwts_option fwts_framework_options[] = {
>   	{ "show-tests-categories","", 0, "Show tests and associated categories." },
>   	{ "acpitests",		"",   0, "Run general ACPI tests." },
>   	{ "acpicompliance",	"",   0, "Run ACPI tests for spec compliance." },
> +	{ "log-level",		"",   1, "Specify error level to report failed test messages," },
>   	{ NULL, NULL, 0, NULL }
>   };
>
> @@ -768,7 +784,8 @@ void fwts_error_inc(fwts_framework *fw, const char *label, int *count)
>    *  fwts_framework_log()
>    *	log a test result
>    */
> -void fwts_framework_log(fwts_framework *fw,
> +void fwts_framework_log(
> +	fwts_framework *fw,
>   	fwts_log_field field,
>   	const char *label,
>   	fwts_log_level level,
> @@ -778,7 +795,7 @@ void fwts_framework_log(fwts_framework *fw,
>   	char buffer[4096];
>   	char prefix[256];
>   	char *str = fwts_log_field_to_str_upper(field);
> -	bool do_count = true;
> +	bool do_count = !FWTS_LEVEL_IGNORE(fw, level);
>
>   	if (fmt) {
>   		va_list ap;
> @@ -797,7 +814,7 @@ void fwts_framework_log(fwts_framework *fw,
>   		} else {
>   			fwts_log_nl(fw);
>   			snprintf(prefix, sizeof(prefix), "%s: ", str);
> -			fwts_log_printf(fw->results, field, level, str, label, prefix, "%s", buffer);
> +			fwts_log_printf(fw, field, level, str, label, prefix, "%s", buffer);
>   			fwts_log_nl(fw);
>   		}
>   		break;
> @@ -812,7 +829,7 @@ void fwts_framework_log(fwts_framework *fw,
>   			fwts_summary_add(fw, fw->current_major_test->name, level, buffer);
>   			snprintf(prefix, sizeof(prefix), "%s [%s] %s: Test %d, ",
>   				str, fwts_log_level_to_str(level), label, fw->current_minor_test_num);
> -			fwts_log_printf(fw->results, field, level, str, label, prefix, "%s", buffer);
> +			fwts_log_printf(fw, field, level, str, label, prefix, "%s", buffer);
>   		}
>   		break;
>   	case LOG_PASSED:
> @@ -821,7 +838,7 @@ void fwts_framework_log(fwts_framework *fw,
>   	case LOG_ABORTED:
>   		snprintf(prefix, sizeof(prefix), "%s: Test %d, ",
>   			str, fw->current_minor_test_num);
> -		fwts_log_printf(fw->results, field, level, str, label, prefix, "%s", buffer);
> +		fwts_log_printf(fw, field, level, str, label, prefix, "%s", buffer);
>   		break;
>   	case LOG_INFOONLY:
>   		break;	/* no-op */
> @@ -1088,6 +1105,28 @@ static int fwts_framework_pm_method_parse(fwts_framework *fw, const char *arg)
>   	return FWTS_OK;
>   }
>
> +/*
> + *  fwts_framework_ll_parse()
> + *	parse log level option
> + */
> +static int fwts_framework_ll_parse(fwts_framework *fw, const char *arg)
> +{
> +	int i;
> +
> +	for (i = 0; log_levels[i].name; i++) {
> +		if (!strcmp(arg, log_levels[i].name)) {
> +			fw->filter_level = log_levels[i].filter_level;
> +			return FWTS_OK;
> +		}
> +	}
> +	fprintf(stderr, "--log-level supports levels:");
> +	for (i = 0; log_levels[i].name; i++)
> +		fprintf(stderr, " %s", log_levels[i].name);
> +	fprintf(stderr, "\n");
> +
> +	return FWTS_ERROR;
> +}
> +
>   int fwts_framework_options_handler(fwts_framework *fw, int argc, char * const argv[], int option_char, int long_index)
>   {
>   	FWTS_UNUSED(argc);
> @@ -1237,6 +1276,10 @@ int fwts_framework_options_handler(fwts_framework *fw, int argc, char * const ar
>   		case 41: /* --acpicompliance */
>   			fw->flags |= FWTS_FLAG_TEST_COMPLIANCE_ACPI;
>   			break;
> +		case 42: /* --log-level */
> +			if (fwts_framework_ll_parse(fw, optarg) != FWTS_OK)
> +				return FWTS_ERROR;
> +			break;
>   		}
>   		break;
>   	case 'a': /* --all */
> @@ -1349,6 +1392,7 @@ int fwts_framework_args(const int argc, char **argv)
>   	fw->flags = FWTS_FLAG_DEFAULT |
>   		    FWTS_FLAG_SHOW_PROGRESS;
>   	fw->log_type = LOG_TYPE_PLAINTEXT;
> +	fw->filter_level = LOG_LEVEL_ALL;
>
>   	fwts_list_init(&fw->errors_filter_keep);
>   	fwts_list_init(&fw->errors_filter_discard);
> diff --git a/src/lib/src/fwts_log.c b/src/lib/src/fwts_log.c
> index fde6035..c526e6c 100644
> --- a/src/lib/src/fwts_log.c
> +++ b/src/lib/src/fwts_log.c
> @@ -393,21 +393,26 @@ static char *fwts_log_filename(const char *filename, const fwts_log_type type)
>   }
>
>   /*
> - *  fwts_log_vprintf()
> + *  fwts_log_printf()
>    *	printf to a log
>    */
> -int fwts_log_printf(fwts_log *log,
> +int fwts_log_printf(
> +	const fwts_framework *fw,
>   	const fwts_log_field field,
>   	const fwts_log_level level,
>   	const char *status,
>   	const char *label,
>   	const char *prefix,
>   	const char *fmt, ...)
> +
>   {
>   	int ret = 0;
> +	fwts_log *log = fw->results;
>
>   	if (!((field & LOG_FIELD_MASK) & fwts_log_filter))
>   		return ret;
> +	if (FWTS_LEVEL_IGNORE(fw, level))
> +		return ret;
>
>   	if (log && log->magic == LOG_MAGIC) {
>   		char buffer[LOG_MAX_BUF_SIZE];
> diff --git a/src/lib/src/fwts_summary.c b/src/lib/src/fwts_summary.c
> index d0fa905..6c2ba92 100644
> --- a/src/lib/src/fwts_summary.c
> +++ b/src/lib/src/fwts_summary.c
> @@ -46,6 +46,14 @@ static const char *summary_names[] = {
>   	"Other"
>   };
>
> +static const int summary_levels[] = {
> +	LOG_LEVEL_CRITICAL,
> +	LOG_LEVEL_HIGH,
> +	LOG_LEVEL_MEDIUM,
> +	LOG_LEVEL_LOW,
> +	LOG_LEVEL_NONE
> +};
> +
>   /* list of summary items per error level */
>   static fwts_list *fwts_summaries[SUMMARY_MAX];
>
> @@ -127,7 +135,8 @@ int fwts_summary_add(
>   	bool summary_item_found = false;
>   	int index = fwts_summary_level_to_index(level);
>
> -	FWTS_UNUSED(fw);
> +	if (FWTS_LEVEL_IGNORE(fw, level))
> +		return FWTS_OK;
>
>   	/* Does the text already exist? - search for it */
>   	fwts_list_foreach(item, fwts_summaries[index]) {
> @@ -186,7 +195,10 @@ int fwts_summary_report(fwts_framework *fw, fwts_list *test_list)
>   	fwts_log_underline(fw->results, '=');
>   	fwts_log_nl(fw);
>
> -	for (i=0;i<SUMMARY_MAX;i++) {
> +	for (i = 0; i < SUMMARY_MAX; i++) {
> +		if (FWTS_LEVEL_IGNORE(fw, summary_levels[i]))
> +			continue;
> +
>   		fwts_log_section_begin(fw->results, "failure");
>
>   		if (fwts_summaries[i]->len) {
>

Acked-by: Ivan Hu <ivan.hu@canonical.com>