Skip to content

Update logging with info on the source file running #531

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Update logging with info on the source file running #531

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4595f64
Update logging with info on the source file running
sven-oly Sep 9, 2025
00bca4b
Add package.json to fix NPM failure
sven-oly Sep 9, 2025
5c8e738
Change hardcoded file names to __file__. Fix typos
sven-oly Oct 2, 2025
b2d7fe7
Small change
sven-oly Oct 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions genData100.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ logrotate -s logrotate.state logrotate.conf

export NVM_DIR=$HOME/.nvm;
source $NVM_DIR/nvm.sh;
# To clear warnings
npm install --package-lock-only
Comment on lines +17 to +18
Copy link
Member

@sffc sffc Sep 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should run npm only in the package directories that need them


#
# Setup
Expand Down
2 changes: 2 additions & 0 deletions generateDataAndRun.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ fi
# Install NVM if it is not install in CI

export NVM_DIR=$HOME/.nvm
# To clear lock
npm install --package-lock-only
Comment on lines +25 to +26
Copy link
Member

@sffc sffc Sep 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same

[ -s "$NVM_DIR/nvm.sh" ] && \. "$NVM_DIR/nvm.sh" # This loads nvm
[ -s "$NVM_DIR/bash_completion" ] && \. "$NVM_DIR/bash_completion" # This loads nvm bash_completion

Expand Down
20 changes: 10 additions & 10 deletions schema/check_generated_data.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ def main(args):

logging.debug('TEST DATA PATH = %s', test_data_path)

logger = logging.Logger("Checking Test Data vs. Schemas LOGGER")
logger = logging.Logger("schema/check_generated_data.py: Checking Test Data vs. Schemas LOGGER")
logger.setLevel(logging.INFO)
logger.info('+++ Test Generated test data vs. schemas files')
logger.info('schema/check_generated_data.py: +++ Test Generated test data vs. schemas')

# TODO: get ICU versions
icu_versions = []
Expand All @@ -39,8 +39,8 @@ def main(args):
for dir_name in icu_dirs:
icu_versions.append(os.path.basename(dir_name))

logging.debug('ICU directories = %s', icu_versions)
logging.debug('test types = %s', ALL_TEST_TYPES)
logging.debug('schema/check_generated_data.py: ICU directories = %s', icu_versions)
logging.debug('schema/check_generated_data.py: test types = %s', ALL_TEST_TYPES)
Comment on lines +42 to +43
Copy link

@gemini-code-assist gemini-code-assist bot Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Hardcoding the filename prefix in log messages, as done throughout this file, is not ideal for maintainability. If the file is ever renamed, all these log strings would need to be updated manually. A better approach is to obtain the filename dynamically. You can define a constant at the top of the module:

import os
FILENAME = os.path.basename(__file__)

And then use it in your log messages, for example:
logging.debug('%s: ICU directories = %%s', FILENAME, icu_versions)


validator = schema_validator.ConformanceSchemaValidator()

Expand All @@ -52,7 +52,7 @@ def main(args):
validator.debug = 1

all_results = validator.validate_test_data_with_schema()
logging.info(' %d results for generated test data', len(all_results))
logging.info('schema/check_generated_data.py: %d results for generated test data', len(all_results))

schema_errors = []
failed_validations = []
Expand All @@ -78,7 +78,7 @@ def main(args):
try:
summary_data = json.dumps(summary_json)
except BaseException as error:
logging.error('json.dumps Summary data problem: %s at %s', error, error)
logging.error('schema/check_generated_data.py: json.dumps Summary data problem: %s at %s', error, error)
sys.exit(1)

output_filename = os.path.join(test_data_path, 'test_data_validation_summary.json')
Expand All @@ -88,17 +88,17 @@ def main(args):
file_out.close()
except BaseException as error:
schema_errors.append(output_filename)
logging.fatal('Error: %s. Cannot save validation summary in file %s', error, output_filename)
logging.fatal('schema/check_generated_data.py: %s. Cannot save validation summary in file %s', error, output_filename)
sys.exit(1)

if schema_errors:
logging.critical('Test data file files: %d fail out of %d:',
logging.critical('schema/check_generated_data.py: Test data file files: %d fail out of %d:',
len(schema_errors), schema_count)
for failure in schema_errors:
logging.critical(' %s', failure)
logging.critical('schema/check_generated_data.py: %s', failure)
sys.exit(1)
else:
logging.info("All %d generated test data files match with schema", schema_count)
logging.info("schema/check_generated_data.py: All %d generated test data files match with schema", schema_count)



Expand Down
16 changes: 8 additions & 8 deletions schema/check_schemas.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def save_schema_validation_summary(self, validation_status):
try:
summary_data = json.dumps(summary_json)
except BaseException as err:
logging.error('%s: Cannot create JSON summary: %s', err, summary_json)
logging.error('schema/check_schemas: %s: Cannot create JSON summary: %s', err, summary_json)
Copy link

@gemini-code-assist gemini-code-assist bot Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Hardcoding the filename prefix in log messages, as done throughout this file, is not ideal for maintainability. If the file is ever renamed, all these log strings would need to be updated manually. A better approach is to obtain the filename dynamically. You can define a constant at the top of the module:

import os
FILENAME = os.path.basename(__file__)

And then use it in your log messages, for example:
logging.error('%s: %%s: Cannot create JSON summary: %%s', FILENAME, err, summary_json)

return None

output_filename = os.path.join(self.schema_base, 'schema_validation_summary.json')
Expand All @@ -54,15 +54,15 @@ def save_schema_validation_summary(self, validation_status):
file_out.write(summary_data)
file_out.close()
except BaseException as error:
logging.warning('Error: %s. Cannot save validation summary in file %s', error, output_filename)
logging.warning('schema/check_schemas: Error: %s. Cannot save validation summary in file %s', error, output_filename)
return None

return output_filename


def parallel_validate_schema(validator, file_names):
num_processors = multiprocessing.cpu_count()
logging.info('Schema validation: %s processors for %s schema validations', num_processors, len(file_names))
logging.info('schema/check_schemas: Schema validation: %s processors for %s schema validations', num_processors, len(file_names))

processor_pool = multiprocessing.Pool(num_processors)
# How to get all the results
Expand All @@ -77,7 +77,7 @@ def parallel_validate_schema(validator, file_names):
def main(args):
logger = logging.Logger("TEST SCHEMAS LOGGER")
logger.setLevel(logging.INFO)
logger.info('+++ Test JSON Schema files')
logger.info('schema/check_schemas: JSON Schema files')

validator = schema_validator.ConformanceSchemaValidator()
# Todo: use setters to initialize validator
Expand Down Expand Up @@ -116,20 +116,20 @@ def main(args):
})
if not result:
schema_errors.append([schema_file, result, err, file_path])
logging.error('Bad Schema at %s', schema_file)
logging.error('schema/check_schemas: Bad Schema at %s', schema_file)
schema_count += 1

output_filename = val_schema.save_schema_validation_summary(validation_status)

if schema_errors:
logging.error('SCHEMA: %d fail out of %d:',
logging.error('schema/check_schemas: SCHEMA: %d fail out of %d:',
len(schema_errors), schema_count)
for failure in schema_errors:
logging.error(' %s', failure)
logging.error('schema/check_schemas: %s', failure)
# We need to clobber the process
sys.exit(1)
else:
logging.info("All %d schema are valid in file %s", schema_count, output_filename)
logging.info("schema/check_schemas: All %d schema are valid in file %s", schema_count, output_filename)
exit(0)


Expand Down
10 changes: 4 additions & 6 deletions schema/check_test_output.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,9 @@ def main(args):
else:
test_output_path = args[1]

logging.debug('TEST OUTPUT PATH = %s', test_output_path)

logger = logging.Logger("Checking Test Data vs. Schemas LOGGER")
logger.setLevel(logging.INFO)
logger.info('+++ Test Generated test data vs. schemas files')
logger.info('+++ schema/check_test_output')

# TODO: get ICU versions
executor_set = set()
Expand Down Expand Up @@ -74,7 +72,7 @@ def main(args):
validator.debug = 1

all_results, test_validation_plans = validator.validate_test_output_with_schema()
logging.info(' %d results for test output', len(all_results))
logging.info('schema/check_test_output: %d results for test output', len(all_results))
Copy link

@gemini-code-assist gemini-code-assist bot Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Hardcoding the filename prefix in log messages, as done throughout this file, is not ideal for maintainability. If the file is ever renamed, all these log strings would need to be updated manually. A better approach is to obtain the filename dynamically. You can define a constant at the top of the module:

import os
FILENAME = os.path.basename(__file__)

And then use it in your log messages, for example:
logging.info('%s: %%d results for test output', FILENAME, len(all_results))


# Check if any files in the expected list were not validated.
test_paths = set()
Expand All @@ -83,7 +81,7 @@ def main(args):

for json_file in json_files:
if json_file not in test_paths:
logging.fatal('JSON file %s was not verified against a schema', json_file)
logging.fatal('schema/check_test_output: JSON file %s was not verified against a schema', json_file)
# Bail out right away!
sys.exit(1)

Expand Down Expand Up @@ -128,7 +126,7 @@ def main(args):
# Don't continue after this problem.
sys.exit(1)

logging.info("All %d test output files match with schema", schema_count)
logging.info("schema/check_test_output: All %d test output files match with schema", schema_count)
return


Expand Down
31 changes: 12 additions & 19 deletions testdriver/testdriver.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,20 +46,20 @@ def set_args(self, arg_options):
for test_type in arg_options.test_type:

if test_type not in ddt_data.testDatasets:
logging.warning('**** WARNING: test_type %s not in testDatasets', test_type)
logging.warning('testdriver.py **** WARNING: test_type %s not in testDatasets', test_type)
Copy link

@gemini-code-assist gemini-code-assist bot Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Hardcoding the filename prefix in log messages, as done throughout this file, is not ideal for maintainability. If the file is ever renamed, all these log strings would need to be updated manually. A better approach is to obtain the filename dynamically. You can define a constant at the top of the module:

import os
FILENAME = os.path.basename(__file__)

And then use it in your log messages, for example:
logging.warning('%s **** WARNING: test_type %%s not in testDatasets', FILENAME, test_type)

else:
# Create a test plan based on data and options
test_data_info = ddt_data.testDatasets[test_type]
if self.debug:
logging.debug('$$$$$ test_type = %s test_data_info = %s',
logging.debug('testdriver.py $$$$$ test_type = %s test_data_info = %s',
test_type, test_data_info.testDataFilename)

for executor in arg_options.exec:
if not ddt_data.allExecutors.has(executor):
# Run a non-specified executor. Compatibility of versions
# between test data and the executor should be done the text executor
# program itself.
logging.error('No executable command configured for executor platform: %s', executor)
logging.error('testdriver.py: No executable command configured for executor platform: %s', executor)
exec_command = {'path': executor}
else:
# Set details for execution from ExecutorInfo
Expand All @@ -76,7 +76,7 @@ def set_args(self, arg_options):
test_data = ddt_data.testDatasets[test_type]
new_plan.set_test_data(test_data)
except KeyError as err:
logging.warning('!!! %s: No test data filename for %s', err, test_type)
logging.warning('testdriver.py !!! %s: No test data filename for %s', err, test_type)

if not new_plan.ignore:
self.test_plans.append(new_plan)
Expand All @@ -91,30 +91,30 @@ def parse_args(self, args):

# Get all the arguments
argparse = ddtargs.DdtArgs(args)
logging.debug('TestDriver OPTIONS: %s', argparse.getOptions())
logging.debug('testdriver.py TestDriver OPTIONS: %s', argparse.getOptions())

# Now use the argparse.options to set the values in the driver
self.set_args(argparse.getOptions())
return

def run_plans(self):
def run_plans(self, logger):
# For each of the plans, run with the appropriate type of parallelism
# Debugging output
logger.info('testdriver.py: running %s plans serially', len(self.test_plans))
for plan in self.test_plans:
plan.run_plan()

def run_one(self, plan):
logging.debug("Parallel of %s %s %s" % (plan.test_lang, plan.test_type, plan.icu_version))
logging.debug("testdriver.py Parallel of %s %s %s" % (plan.test_lang, plan.test_type, plan.icu_version))
plan.run_plan()

def run_plans_parallel(self):
def run_plans_parallel(self, logger):
# Testing 15-Jan-2024
if not self.test_plans or len(self.test_plans) == 0:
return
num_processors = mp.cpu_count()

plan_info = '%s, %s' % (self.test_plans[0].test_type, self.test_plans[0].exec_command)
logging.info('TestDriver: %s processors for %s plans. %s' %
logger.info('testdriver.py TestDriver: %s processors for %s plans. %s' %
(num_processors, len(self.test_plans), plan_info))

processor_pool = mp.Pool(num_processors)
Expand All @@ -133,16 +133,9 @@ def main(args):
logger.setLevel(logging.INFO)

if driver.run_serial:
driver.run_plans()
driver.run_plans(logger)
else:
driver.run_plans_parallel()

# if len(args)> 2:
# Set limit on number to run
# numberToRun = int(args[2])
# driver.runLimit = numberToRun

# driver.initExecutor()
driver.run_plans_parallel(logger)


if __name__ == "__main__":
Expand Down
Loading
Loading