From d60702c7ec86b4b8d718c6bbf2955052a1ff23ea Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Tue, 27 Aug 2019 15:02:03 -0700 Subject: [PATCH 1/7] Initial commit --- test/model_level_tests/test_main.py | 37 ++++++++++++++++++++--------- test/python/test_modeltester.py | 2 +- 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/test/model_level_tests/test_main.py b/test/model_level_tests/test_main.py index 989f66142..492669c38 100644 --- a/test/model_level_tests/test_main.py +++ b/test/model_level_tests/test_main.py @@ -130,7 +130,8 @@ def ready_repo(model_dir, repo_dl_loc): # TODO: this function needs to accept "do-i-dump-pbtxt"? and if so, a cleanup needs to happen later. # Also this function could return the list of pbtxts it generated (but does it need to? we can infer it) # TODO: this function should also take the level/intensity of test to run -def run_test_suite(model_dir, configuration, disabled, print_parsed): +def run_test_suite(model_dir, configuration, disabled, print_parsed, + ignore_test): try: # TODO: assert TF version. Some models may not run on TF1.12 etc model_dir = os.path.abspath(model_dir) @@ -239,14 +240,15 @@ def run_test_suite(model_dir, configuration, disabled, print_parsed): not custom_parser_present) except: assert False, 'Failed to parse ' + expected_json_file - passed, fail_help_string = compare_parsed_values( - parsed_vals, expected.get('logparse', {})) - if not passed: - print('Failed in test ' + flname + - '. Help message: ' + fail_help_string) - failed_tests.append(flname) - continue - if 'time' in expected: + if 'logparse' in expected and 'logparse' not in ignore_test: + passed, fail_help_string = compare_parsed_values( + parsed_vals, expected.get('logparse', {})) + if not passed: + print('Failed in test ' + flname + + '. Help message: ' + fail_help_string) + failed_tests.append(flname) + continue + if 'time' in expected and 'time' not in ignore_test: actual_runtime = tend - tstart # TODO: decide this criteria. time can be pretty variable # TODO: the percentage (0.1) for the time bound might be passed through `expected.json` @@ -372,7 +374,7 @@ def get_disabled_tests_info(): action='store', type=str, help='Comma separated list of model names', - default='') + ) parser.add_argument( '--list', action='store_true', @@ -399,6 +401,13 @@ def get_disabled_tests_info(): help= 'Print the parsed values from log parsing. Useful when checking in a new model and we want to know its expected values' ) + parser.add_argument( + '--ignore_test', + type=str, + default='', + help= + 'Comma separated string. Given an expected json file, ignore these tests. Can take values "", "logparse", "time", "logparse,time", "time,logparse"' + ) # This script must be run from this location assert cwd.split('/')[-1] == 'model_level_tests' @@ -419,6 +428,11 @@ def get_disabled_tests_info(): args.run_basic_tests or args.run_functional_tests ), 'No type of test enabled. Please choose --run_basic_tests, --run_functional_tests or both' + ignore_test = args.ignore_test.split(',') + assert ( + all(map(lambda i: i in ['time', 'logparse'], ignore_test)) + ), "2 types of tests are possible which can be skipped: logparse or time, but requested to skip " + args.ignore_test + requested_test_suites = os.listdir( 'models') if args.models == '' else args.models.split(',') @@ -440,7 +454,8 @@ def get_disabled_tests_info(): if args.run_basic_tests: passed_tests_in_suite, failed_tests_in_suite, skipped_tests_in_suite = run_test_suite( './models/' + test_suite, args.configuration, - disabled_sub_test.get(test_suite, []), args.print_parsed) + disabled_sub_test.get(test_suite, []), args.print_parsed, + ignore_test) passed_tests[test_suite] = passed_tests_in_suite failed_tests[test_suite] = failed_tests_in_suite skipped_tests[test_suite] = skipped_tests_in_suite diff --git a/test/python/test_modeltester.py b/test/python/test_modeltester.py index 3bb3c4b0a..1cab4abac 100644 --- a/test/python/test_modeltester.py +++ b/test/python/test_modeltester.py @@ -47,7 +47,7 @@ def test_MLP(self): config = "varopts" if varopts else "default" try: command_executor( - "python test_main.py --run_basic_tests --models MLP --configuration " + "python test_main.py --run_basic_tests --models MLP --ignore_test time --configuration " + config) finally: os.chdir(cwd) From 62fe6fc60201fef4ac01d5775a5dd7cc031a089f Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Tue, 27 Aug 2019 15:06:09 -0700 Subject: [PATCH 2/7] Update test/model_level_tests/test_main.py --- test/model_level_tests/test_main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/model_level_tests/test_main.py b/test/model_level_tests/test_main.py index 492669c38..3ae3b979d 100644 --- a/test/model_level_tests/test_main.py +++ b/test/model_level_tests/test_main.py @@ -242,7 +242,7 @@ def run_test_suite(model_dir, configuration, disabled, print_parsed, assert False, 'Failed to parse ' + expected_json_file if 'logparse' in expected and 'logparse' not in ignore_test: passed, fail_help_string = compare_parsed_values( - parsed_vals, expected.get('logparse', {})) + parsed_vals, expected['logparse']) if not passed: print('Failed in test ' + flname + '. Help message: ' + fail_help_string) From 9152260d47dc3d3e344552bc0e80fdaaed9804b6 Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Tue, 3 Sep 2019 18:56:22 -0700 Subject: [PATCH 3/7] Add comments and a couple of new helper functions --- test/model_level_tests/test_main.py | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/test/model_level_tests/test_main.py b/test/model_level_tests/test_main.py index 3ae3b979d..b6d2b225a 100644 --- a/test/model_level_tests/test_main.py +++ b/test/model_level_tests/test_main.py @@ -127,6 +127,16 @@ def ready_repo(model_dir, repo_dl_loc): command_executor(model_dir + '/getting_repo_ready.sh', verbose=True) +# Currently there are 2 types of tests. parsing the logs and timing the run +def valid_test_types(): + return set(['time', 'logparse']) + + +# Check if the contents of this iterable contains only valid test types +def check_test_types(iterable): + return all(map(lambda i: i in valid_test_types(), iterable)) + + # TODO: this function needs to accept "do-i-dump-pbtxt"? and if so, a cleanup needs to happen later. # Also this function could return the list of pbtxts it generated (but does it need to? we can infer it) # TODO: this function should also take the level/intensity of test to run @@ -240,6 +250,11 @@ def run_test_suite(model_dir, configuration, disabled, print_parsed, not custom_parser_present) except: assert False, 'Failed to parse ' + expected_json_file + assert check_test_types(expected.keys( + )), "Got unexpected key in " + expected.keys( + ) + ". Should have been " + ','.join(valid_test_types) + # We run the test if 'logparse' is present in the expected values to check + # for and it is not in the ignore list if 'logparse' in expected and 'logparse' not in ignore_test: passed, fail_help_string = compare_parsed_values( parsed_vals, expected['logparse']) @@ -429,9 +444,9 @@ def get_disabled_tests_info(): ), 'No type of test enabled. Please choose --run_basic_tests, --run_functional_tests or both' ignore_test = args.ignore_test.split(',') - assert ( - all(map(lambda i: i in ['time', 'logparse'], ignore_test)) - ), "2 types of tests are possible which can be skipped: logparse or time, but requested to skip " + args.ignore_test + assert (check_test_types(ignore_test) + ), "Types of possible tests: " + ','.join(valid_test_types()) + + ", but requested to skip " + args.ignore_test requested_test_suites = os.listdir( 'models') if args.models == '' else args.models.split(',') From ad6bd27b14670735ee250d960eafd2bab5256ccd Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Wed, 4 Sep 2019 13:47:55 -0700 Subject: [PATCH 4/7] Add brackets --- test/model_level_tests/test_main.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/model_level_tests/test_main.py b/test/model_level_tests/test_main.py index b6d2b225a..ff3df60b4 100644 --- a/test/model_level_tests/test_main.py +++ b/test/model_level_tests/test_main.py @@ -255,7 +255,8 @@ def run_test_suite(model_dir, configuration, disabled, print_parsed, ) + ". Should have been " + ','.join(valid_test_types) # We run the test if 'logparse' is present in the expected values to check # for and it is not in the ignore list - if 'logparse' in expected and 'logparse' not in ignore_test: + if ('logparse' in expected) and ( + 'logparse' not in ignore_test): passed, fail_help_string = compare_parsed_values( parsed_vals, expected['logparse']) if not passed: @@ -263,7 +264,7 @@ def run_test_suite(model_dir, configuration, disabled, print_parsed, '. Help message: ' + fail_help_string) failed_tests.append(flname) continue - if 'time' in expected and 'time' not in ignore_test: + if ('time' in expected) and ('time' not in ignore_test): actual_runtime = tend - tstart # TODO: decide this criteria. time can be pretty variable # TODO: the percentage (0.1) for the time bound might be passed through `expected.json` From fc69bdecfc8ea9cf4713a6a7a89bd9f02cf2fb62 Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Wed, 4 Sep 2019 14:31:52 -0700 Subject: [PATCH 5/7] Fix syntax error --- test/model_level_tests/test_main.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/model_level_tests/test_main.py b/test/model_level_tests/test_main.py index ff3df60b4..77f0e6804 100644 --- a/test/model_level_tests/test_main.py +++ b/test/model_level_tests/test_main.py @@ -446,8 +446,8 @@ def get_disabled_tests_info(): ignore_test = args.ignore_test.split(',') assert (check_test_types(ignore_test) - ), "Types of possible tests: " + ','.join(valid_test_types()) - + ", but requested to skip " + args.ignore_test + ), "Types of possible tests: " + ','.join(valid_test_types()) + \ + ", but requested to skip " + args.ignore_test requested_test_suites = os.listdir( 'models') if args.models == '' else args.models.split(',') From f22bcf31a23fb10e9926d4a3f08a1f786c2badc2 Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Fri, 6 Sep 2019 10:39:29 -0700 Subject: [PATCH 6/7] Minor fix --- test/model_level_tests/test_main.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/model_level_tests/test_main.py b/test/model_level_tests/test_main.py index 77f0e6804..41a8acb21 100644 --- a/test/model_level_tests/test_main.py +++ b/test/model_level_tests/test_main.py @@ -420,7 +420,7 @@ def get_disabled_tests_info(): parser.add_argument( '--ignore_test', type=str, - default='', + default=None, help= 'Comma separated string. Given an expected json file, ignore these tests. Can take values "", "logparse", "time", "logparse,time", "time,logparse"' ) @@ -444,8 +444,8 @@ def get_disabled_tests_info(): args.run_basic_tests or args.run_functional_tests ), 'No type of test enabled. Please choose --run_basic_tests, --run_functional_tests or both' - ignore_test = args.ignore_test.split(',') - assert (check_test_types(ignore_test) + ignore_test = [] if (args.ignore_test is None) else args.ignore_test.split(',') + assert ((ignore_test=='') or check_test_types(ignore_test) ), "Types of possible tests: " + ','.join(valid_test_types()) + \ ", but requested to skip " + args.ignore_test From 31b782292cc722438a93710d5d8e682ec6aa5c25 Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Fri, 6 Sep 2019 10:40:55 -0700 Subject: [PATCH 7/7] Style --- test/model_level_tests/test_main.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/model_level_tests/test_main.py b/test/model_level_tests/test_main.py index 41a8acb21..66fa6443d 100644 --- a/test/model_level_tests/test_main.py +++ b/test/model_level_tests/test_main.py @@ -444,7 +444,8 @@ def get_disabled_tests_info(): args.run_basic_tests or args.run_functional_tests ), 'No type of test enabled. Please choose --run_basic_tests, --run_functional_tests or both' - ignore_test = [] if (args.ignore_test is None) else args.ignore_test.split(',') + ignore_test = [] if ( + args.ignore_test is None) else args.ignore_test.split(',') assert ((ignore_test=='') or check_test_types(ignore_test) ), "Types of possible tests: " + ','.join(valid_test_types()) + \ ", but requested to skip " + args.ignore_test