| Message ID | 20201017180749.119271-22-seanga2@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | log: Add commands for manipulating filters | expand |
On Sat, Oct 17, 2020 at 02:07:46PM -0400, Sean Anderson wrote: > This exercises a few success and failure modes of the log filter-* > commands. log filter-list is not tested because it's purely informational. > I don't think there's a good way to test it except by testing if the output > of the command exactly matches a sample run. > > Signed-off-by: Sean Anderson <seanga2@gmail.com> > Reviewed-by: Simon Glass <sjg@chromium.org> Building this with Clang shows a real problem. A full log is over at https://dev.azure.com/u-boot/u-boot/_build/results?buildId=1386&view=logs&j=f22b025e-3f3e-5478-618e-bef68154f752&t=0594e91d-c1b1-5d5d-b353-a764bbd01b55 but: /home/trini/u-boot/u-boot/test/log/log_filter.c:64:13: warning: implicit conversion from enumeration type 'enum uclass_id' to different enumeration type 'enum log_category_t' [-Wenum-conversion] UCLASS_MMC)); ^~~~~~~~~~ /home/trini/u-boot/u-boot/include/test/ut.h:124:41: note: expanded from macro 'ut_asserteq' unsigned int _val1 = (expr1), _val2 = (expr2); \ ^~~~~ /home/trini/u-boot/u-boot/test/log/log_filter.c:66:13: warning: implicit conversion from enumeration type 'enum uclass_id' to different enumeration type 'enum log_category_t' [-Wenum-conversion] UCLASS_SPI)); ^~~~~~~~~~ /home/trini/u-boot/u-boot/include/test/ut.h:124:41: note: expanded from macro 'ut_asserteq' unsigned int _val1 = (expr1), _val2 = (expr2); \ ^~~~~ 2 warnings generated. Was perhaps an earlier series, or some other series, adding LOGC_MMC / LOGC_SPI ? Thanks!
On 10/27/20 5:00 PM, Tom Rini wrote: > On Sat, Oct 17, 2020 at 02:07:46PM -0400, Sean Anderson wrote: > >> This exercises a few success and failure modes of the log filter-* >> commands. log filter-list is not tested because it's purely informational. >> I don't think there's a good way to test it except by testing if the output >> of the command exactly matches a sample run. >> >> Signed-off-by: Sean Anderson <seanga2@gmail.com> >> Reviewed-by: Simon Glass <sjg@chromium.org> > > Building this with Clang shows a real problem. A full log is over at > https://dev.azure.com/u-boot/u-boot/_build/results?buildId=1386&view=logs&j=f22b025e-3f3e-5478-618e-bef68154f752&t=0594e91d-c1b1-5d5d-b353-a764bbd01b55 > but: > /home/trini/u-boot/u-boot/test/log/log_filter.c:64:13: warning: implicit conversion from enumeration type 'enum uclass_id' to different enumeration type 'enum log_category_t' [-Wenum-conversion] > UCLASS_MMC)); > ^~~~~~~~~~ > /home/trini/u-boot/u-boot/include/test/ut.h:124:41: note: expanded from macro 'ut_asserteq' > unsigned int _val1 = (expr1), _val2 = (expr2); \ > ^~~~~ > /home/trini/u-boot/u-boot/test/log/log_filter.c:66:13: warning: implicit conversion from enumeration type 'enum uclass_id' to different enumeration type 'enum log_category_t' [-Wenum-conversion] > UCLASS_SPI)); > ^~~~~~~~~~ > /home/trini/u-boot/u-boot/include/test/ut.h:124:41: note: expanded from macro 'ut_asserteq' > unsigned int _val1 = (expr1), _val2 = (expr2); \ > ^~~~~ > 2 warnings generated. > > Was perhaps an earlier series, or some other series, adding LOGC_MMC / > LOGC_SPI ? Thanks! > I think it's because UCLASS_SPI is an enum uclass_t, and those functions expect a enum log_category_t. They should be cast with log_uc_cat. I can send a v4 fixing this. --Sean
On Tue, Oct 27, 2020 at 05:11:58PM -0400, Sean Anderson wrote: > > On 10/27/20 5:00 PM, Tom Rini wrote: > > On Sat, Oct 17, 2020 at 02:07:46PM -0400, Sean Anderson wrote: > > > >> This exercises a few success and failure modes of the log filter-* > >> commands. log filter-list is not tested because it's purely informational. > >> I don't think there's a good way to test it except by testing if the output > >> of the command exactly matches a sample run. > >> > >> Signed-off-by: Sean Anderson <seanga2@gmail.com> > >> Reviewed-by: Simon Glass <sjg@chromium.org> > > > > Building this with Clang shows a real problem. A full log is over at > > https://dev.azure.com/u-boot/u-boot/_build/results?buildId=1386&view=logs&j=f22b025e-3f3e-5478-618e-bef68154f752&t=0594e91d-c1b1-5d5d-b353-a764bbd01b55 > > but: > > /home/trini/u-boot/u-boot/test/log/log_filter.c:64:13: warning: implicit conversion from enumeration type 'enum uclass_id' to different enumeration type 'enum log_category_t' [-Wenum-conversion] > > UCLASS_MMC)); > > ^~~~~~~~~~ > > /home/trini/u-boot/u-boot/include/test/ut.h:124:41: note: expanded from macro 'ut_asserteq' > > unsigned int _val1 = (expr1), _val2 = (expr2); \ > > ^~~~~ > > /home/trini/u-boot/u-boot/test/log/log_filter.c:66:13: warning: implicit conversion from enumeration type 'enum uclass_id' to different enumeration type 'enum log_category_t' [-Wenum-conversion] > > UCLASS_SPI)); > > ^~~~~~~~~~ > > /home/trini/u-boot/u-boot/include/test/ut.h:124:41: note: expanded from macro 'ut_asserteq' > > unsigned int _val1 = (expr1), _val2 = (expr2); \ > > ^~~~~ > > 2 warnings generated. > > > > Was perhaps an earlier series, or some other series, adding LOGC_MMC / > > LOGC_SPI ? Thanks! > > > > I think it's because UCLASS_SPI is an enum uclass_t, and those functions > expect a enum log_category_t. They should be cast with log_uc_cat. I can > send a v4 fixing this. OK, thanks.
diff --git a/include/test/log.h b/include/test/log.h index 772e197806..e902891450 100644 --- a/include/test/log.h +++ b/include/test/log.h @@ -14,5 +14,6 @@ /* Declare a new logging test */ #define LOG_TEST(_name) UNIT_TEST(_name, 0, log_test) +#define LOG_TEST_FLAGS(_name, _flags) UNIT_TEST(_name, _flags, log_test) #endif /* __TEST_LOG_H__ */ diff --git a/test/log/Makefile b/test/log/Makefile index 52e2f7b41c..9a6cdbe643 100644 --- a/test/log/Makefile +++ b/test/log/Makefile @@ -3,6 +3,7 @@ # Copyright (c) 2017 Google, Inc obj-$(CONFIG_LOG_TEST) += log_test.o +obj-$(CONFIG_CMD_LOG) += log_filter.o ifdef CONFIG_UT_LOG diff --git a/test/log/log_filter.c b/test/log/log_filter.c new file mode 100644 index 0000000000..e27229dbd9 --- /dev/null +++ b/test/log/log_filter.c @@ -0,0 +1,108 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2020 Sean Anderson <seanga2@gmail.com> + */ + +#include <common.h> +#include <console.h> +#include <log.h> +#include <test/log.h> +#include <test/ut.h> + +DECLARE_GLOBAL_DATA_PTR; + +/* Test invalid options */ +static int log_test_filter_invalid(struct unit_test_state *uts) +{ + ut_asserteq(1, run_command("log filter-add -AD", 0)); + ut_asserteq(1, run_command("log filter-add -l1 -L1", 0)); + ut_asserteq(1, run_command("log filter-add -l1 -L1", 0)); + ut_asserteq(1, run_command("log filter-add -lfoo", 0)); + ut_asserteq(1, run_command("log filter-add -cfoo", 0)); + ut_asserteq(1, run_command("log filter-add -ccore -ccore -ccore -ccore " + "-ccore -ccore", 0)); + + return 0; +} +LOG_TEST_FLAGS(log_test_filter_invalid, UT_TESTF_CONSOLE_REC); + +/* Test adding and removing filters */ +static int log_test_filter(struct unit_test_state *uts) +{ + bool any_found = false; + bool filt1_found = false; + bool filt2_found = false; + char cmd[32]; + struct log_filter *filt; + struct log_device *ldev; + ulong filt1, filt2; + +#define create_filter(args, filter_num) do {\ + ut_assertok(console_record_reset_enable()); \ + ut_assertok(run_command("log filter-add -p " args, 0)); \ + ut_assert_skipline(); \ + ut_assertok(strict_strtoul(uts->actual_str, 10, &(filter_num))); \ + ut_assert_console_end(); \ +} while (0) + + create_filter("", filt1); + create_filter("-DL warning -cmmc -cspi -ffile", filt2); + + ldev = log_device_find_by_name("console"); + ut_assertnonnull(ldev); + list_for_each_entry(filt, &ldev->filter_head, sibling_node) { + if (filt->filter_num == filt1) { + filt1_found = true; + ut_asserteq(0, filt->flags); + ut_asserteq(LOGL_MAX, filt->level); + ut_assertnull(filt->file_list); + } else if (filt->filter_num == filt2) { + filt2_found = true; + ut_asserteq(LOGFF_HAS_CAT | LOGFF_DENY | + LOGFF_LEVEL_MIN, filt->flags); + ut_asserteq(true, log_has_cat(filt->cat_list, + UCLASS_MMC)); + ut_asserteq(true, log_has_cat(filt->cat_list, + UCLASS_SPI)); + ut_asserteq(LOGL_WARNING, filt->level); + ut_asserteq_str("file", filt->file_list); + } + } + ut_asserteq(true, filt1_found); + ut_asserteq(true, filt2_found); + +#define remove_filter(filter_num) do { \ + ut_assertok(console_record_reset_enable()); \ + snprintf(cmd, sizeof(cmd), "log filter-remove %lu", filter_num); \ + ut_assertok(run_command(cmd, 0)); \ + ut_assert_console_end(); \ +} while (0) + + remove_filter(filt1); + remove_filter(filt2); + + filt1_found = false; + filt2_found = false; + list_for_each_entry(filt, &ldev->filter_head, sibling_node) { + if (filt->filter_num == filt1) + filt1_found = true; + else if (filt->filter_num == filt2) + filt2_found = true; + } + ut_asserteq(false, filt1_found); + ut_asserteq(false, filt2_found); + + create_filter("", filt1); + create_filter("", filt2); + + ut_assertok(console_record_reset_enable()); + ut_assertok(run_command("log filter-remove -a", 0)); + ut_assert_console_end(); + + list_for_each_entry(filt, &ldev->filter_head, sibling_node) + any_found = true; + ut_asserteq(false, any_found); + + return 0; +} +LOG_TEST_FLAGS(log_test_filter, UT_TESTF_CONSOLE_REC);