[PATCH 1/3] lib: fwts_framework, fwts_klog: Add filtering to fwts errors

IvanHu ivan.hu at canonical.com
Wed Nov 28 09:06:58 UTC 2012


On 11/23/2012 12:30 AM, Colin King wrote:
> From: Colin Ian King <colin.king at canonical.com>
>
> Certification would like to be able to filter some of the fwts errors
> so that they can skip over some known false positives on some hardware.
>
> So I'm introducing two new fwts options:
>
> --filter-error-discard
> --filter-error-keep
>
> How do these work?
>
> --filter-error-discard is supplied a comma separated list of fwts
> error labels that you don't want reported, e.g.
>
> fwts s3 --filter-error-discard=DevConfigDiffAfterS3,ShortSuspend
>
> ..and fwts will run the test but not report these as errors. It
> will effectively silently discard these.  So it is an "opt-out" mechanism.
>
> Alternatively, one can specify the errors you just want reported, e.g.
>
> fwts s3 --filter-error-keep=BadWakeAlarmS3,DevConfigDiffAfterS3
>
> ..so fwts will run the test and *only* report these errors.  So this
> states which test errors you want to opt-in to test.
>
> These can be abused, and probably will. One can make fwts X
> pass all the tests using:
>
> fwts --filter-error-keep=foo
>
> ..since there are no error labels "foo" in fwts.  I am opening up
> a can of worms, since we provide a mechanism to make tests magically
> pass when fwts is finding errors.  But really like all QA tools, if
> somebody wants to abuse the results they can and they will. This
> just makes it easier for them to do.
>
> Signed-off-by: Colin Ian King <colin.king at canonical.com>
> ---
>   src/lib/include/fwts_framework.h |   5 ++
>   src/lib/src/fwts_framework.c     | 114 ++++++++++++++++++++++++++++++++++-----
>   src/lib/src/fwts_klog.c          |   2 +-
>   3 files changed, 108 insertions(+), 13 deletions(-)
>
> diff --git a/src/lib/include/fwts_framework.h b/src/lib/include/fwts_framework.h
> index ea40a07..2723ccb 100644
> --- a/src/lib/include/fwts_framework.h
> +++ b/src/lib/include/fwts_framework.h
> @@ -138,6 +138,10 @@ typedef struct {
>   	bool show_progress;			/* Show progress while running current test */
>
>   	fwts_log_type	log_type;		/* Output log type, default is plain text ASCII */
> +
> +	fwts_list errors_filter_keep;		/* Results to keep, empty = keep all */
> +	fwts_list errors_filter_discard;	/* Results to discard, empty = discard none */
> +	bool error_filtered_out;		/* True if a klog message has been filtered out */
>   } fwts_framework;
>
>   typedef struct {
> @@ -194,6 +198,7 @@ void fwts_framework_aborted(fwts_framework *, const char *fmt, ...)
>   	__attribute__((format(printf, 2, 3)));
>   void fwts_framework_infoonly(fwts_framework *fw);
>   void fwts_framework_minor_test_progress(fwts_framework *fw, const int percent, const char *message);
> +void fwts_error_inc(fwts_framework *fw, const char *label, int *count);
>
>   void fwts_framework_log(fwts_framework *fw,
>   	fwts_log_field field,
> diff --git a/src/lib/src/fwts_framework.c b/src/lib/src/fwts_framework.c
> index 00b9ed7..1f01466 100644
> --- a/src/lib/src/fwts_framework.c
> +++ b/src/lib/src/fwts_framework.c
> @@ -80,6 +80,8 @@ static fwts_option fwts_framework_options[] = {
>   	{ "disassemble-aml", 	"",   0, "Disassemble AML from DSDT and SSDT tables." },
>   	{ "log-type",		"",   1, "Specify log type (plaintext, json, html or xml)." },
>   	{ "unsafe",		"U",  0, "Unsafe tests (tests that can potentially cause kernel oopses." },
> +	{ "filter-error-discard", "", 1, "Discard errors that match any of the specified labels." },
> +	{ "filter-error-keep",	"",   1, "Keep errors that match any of the specified labels." },
>   	{ NULL, NULL, 0, NULL }
>   };
>
> @@ -649,6 +651,50 @@ static fwts_framework_test *fwts_framework_test_find(const char *name)
>   	return NULL;
>   }
>
> +bool fwts_error_filtered_out(fwts_framework *fw, const char *label)
> +{
> +	fwts_list_link *item;
> +
> +	/*
> +	 *  Has the user specified errors to discard?  If we find any matches
> +	 *  then flag as wanting to filter out.
> +	 */
> +	if (fwts_list_len(&fw->errors_filter_discard) > 0) {
> +		fwts_list_foreach(item, &fw->errors_filter_discard) {
> +			if (strcmp(label, fwts_list_data(char *, item)) == 0)
> +				return true;	/* Discard */
> +		}
> +		return false;	/* No matches, won't discard */
> +	}
> +
> +	/*
> +	 *  Has the user specified errors to keep?  If we find any matches
> +	 *  then flag as wanting to keep.
> +	 */
> +	if (fwts_list_len(&fw->errors_filter_keep) > 0) {
> +		fwts_list_foreach(item, &fw->errors_filter_keep) {
> +			if (strcmp(label, fwts_list_data(char *, item)) == 0)
> +				return false;	/* Don't discard */
> +		}
> +		return true;	/* Not found, so discard */
> +	}
> +
> +	/*
> +	 *  User not specified any filters?  Don't discard
> +	 */
> +	return false;
> +}
> +
> +/*
> + *  fwts_error_inc()
> + *	Increment the error count if we're not filtering out this error (based on label).
> + */
> +void fwts_error_inc(fwts_framework *fw, const char *label, int *count)
> +{
> +	if (!fwts_error_filtered_out(fw, label))
> +		(*count)++;
> +}
> +
>   /*
>    *  fwts_framework_log()
>    *	log a test result
> @@ -663,6 +709,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;
>
>   	if (fmt) {
>   		va_list ap;
> @@ -673,22 +720,31 @@ void fwts_framework_log(fwts_framework *fw,
>   	} else
>   		*buffer = '\0';
>
> -	if (count)
> -		(*count)++;
> -
>   	switch (field) {
>   	case LOG_ADVICE:
> -		fwts_log_nl(fw);
> -		snprintf(prefix, sizeof(prefix), "%s: ", str);
> -		fwts_log_printf(fw->results, field, level, str, label, prefix, "%s", buffer);
> -		fwts_log_nl(fw);
> +		/* If the previous LOG_FAILED message was filtered out, ignore following advice */
> +		if (fw->error_filtered_out) {
> +			do_count = false;
> +		} 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_nl(fw);
> +		}
>   		break;
>   	case LOG_FAILED:
> -		fw->failed_level |= level;
> -		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);
> +		if (fwts_error_filtered_out(fw, label)) {
> +			fw->error_filtered_out = true;
> +			do_count = false;
> +		} else {
> +			fw->error_filtered_out = false;
> +
> +			fw->failed_level |= level;
> +			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);
> +		}
>   		break;
>   	case LOG_PASSED:
>   	case LOG_WARNING:
> @@ -703,6 +759,10 @@ void fwts_framework_log(fwts_framework *fw,
>   	default:
>   		break;
>   	}
> +
> +	/* Only increment stats if we've reported something */
> +	if (do_count && (count != NULL))
> +		(*count)++;
>   }
>
>   /*
> @@ -836,6 +896,23 @@ static int fwts_framework_skip_test_parse(const char *arg, fwts_list *tests_to_s
>   	return FWTS_OK;
>   }
>
> +static int fwts_framework_filter_error_parse(const char *arg, fwts_list *list)
> +{
> +	char *str;
> +	char *token;
> +	char *saveptr = NULL;
> +
> +	for (str = (char*)arg; (token = strtok_r(str, ",", &saveptr)) != NULL; str = NULL) {
> +		if (fwts_list_append(list, token) == NULL) {
> +			fprintf(stderr, "Out of memory parsing argument %s\n", arg);
> +			fwts_list_free(list, NULL);
> +			return FWTS_ERROR;
> +		}
> +	}
> +
> +	return FWTS_OK;
> +}
> +
>   /*
>    *  fwts_framework_log_type_parse()
>    *	parse optarg of comma separated log types
> @@ -992,6 +1069,14 @@ int fwts_framework_options_handler(fwts_framework *fw, int argc, char * const ar
>   		case 33: /* --unsafe */
>   			fw->flags |= FWTS_FLAG_UNSAFE;
>   			break;
> +		case 34: /* --filter-error-discard */
> +			if (fwts_framework_filter_error_parse(optarg, &fw->errors_filter_discard) != FWTS_OK)
> +				return FWTS_ERROR;
> +			break;
> +		case 35: /* --filter-error-keep */
> +			if (fwts_framework_filter_error_parse(optarg, &fw->errors_filter_keep) != FWTS_OK)
> +				return FWTS_ERROR;
> +			break;
>   		}
>   		break;
>   	case 'a': /* --all */
> @@ -1100,6 +1185,8 @@ int fwts_framework_args(const int argc, char **argv)
>   	fw->log_type = LOG_TYPE_PLAINTEXT;
>
>   	fwts_list_init(&fw->total_taglist);
> +	fwts_list_init(&fw->errors_filter_keep);
> +	fwts_list_init(&fw->errors_filter_discard);
>
>   	fwts_summary_init();
>
> @@ -1243,6 +1330,9 @@ tidy_close:
>   	free(fw->results_logname);
>   	free(fw->klog);
>   	free(fw->json_data_path);
> +
> +	fwts_list_free_items(&fw->errors_filter_discard, NULL);
> +	fwts_list_free_items(&fw->errors_filter_keep, NULL);
>   	fwts_list_free_items(&fw->total_taglist, free);
>   	fwts_list_free_items(&fwts_framework_test_list, free);
>
> diff --git a/src/lib/src/fwts_klog.c b/src/lib/src/fwts_klog.c
> index 0a6350c..7d75b6d 100644
> --- a/src/lib/src/fwts_klog.c
> +++ b/src/lib/src/fwts_klog.c
> @@ -255,7 +255,7 @@ void fwts_klog_scan_patterns(fwts_framework *fw,
>   				fwts_tag_failed(fw, pattern->tag);
>   				fwts_failed(fw, pattern->level, pattern->label,
>   					"%s Kernel message: %s", fwts_log_level_to_str(pattern->level), line);
> -				(*errors)++;
> +				fwts_error_inc(fw, pattern->label, errors);
>   			}
>   			if (repeated)
>   				fwts_log_info(fw, "Message repeated %d times.", repeated);
>
Acked-by: Ivan Hu <ivan.hu at canonical.com>




More information about the fwts-devel mailing list