Hi Ilpo, On 4/27/2023 1:04 AM, Ilpo Järvinen wrote: > On Wed, 26 Apr 2023, Reinette Chatre wrote: >> On 4/26/2023 6:58 AM, Ilpo Järvinen wrote: >>> On Fri, 21 Apr 2023, Reinette Chatre wrote: >>>> On 4/18/2023 4:45 AM, Ilpo Järvinen wrote: >> >> ... >> >>>>> diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c >>>>> index 4b505fdb35d7..85053829b9c5 100644 >>>>> --- a/tools/testing/selftests/resctrl/cat_test.c >>>>> +++ b/tools/testing/selftests/resctrl/cat_test.c >>>>> @@ -11,11 +11,12 @@ >>>>> #include "resctrl.h" >>>>> #include <unistd.h> >>>>> >>>>> -#define RESULT_FILE_NAME1 "result_cat1" >>>>> -#define RESULT_FILE_NAME2 "result_cat2" >>>>> -#define NUM_OF_RUNS 5 >>>>> -#define MAX_DIFF_PERCENT 4 >>>>> -#define MAX_DIFF 1000000 >>>>> +#define RESULT_FILE_NAME "result_cat" >>>>> +#define NUM_OF_RUNS 5 >>>>> +#define MIN_DIFF_PERCENT_PER_BIT 2 >>>> >>>> Could you please start a new trend that adds documentation >>>> that explains what this constant means and how it was chosen? >>> >>> I can try although that particular 2 was a bit handwavy that just seems to >>> work with the tests I performed. >> >> The changelog claims that the existing CAT test does not work with >> this new test offered as replacement. Considering that I do think it >> is important to have confidence that this test is able to test CAT. >> The words "handwave" and "seems to work" are red flags to me. >> When merged, these tests will be run on a variety of platforms with >> various configurations. Using test criteria based on measurements >> from one particular system may work but there needs to be confidence >> that the criteria maps to all systems these tests will be run on. > > My "tests" (in plural) were not limited to one particular system but > included systems from different generations. > Thank you very much for your thorough testing. Having this information accompany this change will surely help to increase confidence in the value chosen. Thank you very much Reinette
