ACK: [PATCH 1/2] fwts: framework: Add --log-level option
Alex Hung
alex.hung at canonical.com
Tue Dec 15 14:42:53 UTC 2015
On 2015-12-10 08:20 PM, Colin King wrote:
> From: Colin Ian King <colin.king at 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 at 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 at canonical.com>
More information about the fwts-devel
mailing list