Skip to content
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.

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

Custom qa-docs logger use #1896

Merged
merged 15 commits into from
Sep 16, 2021
Merged

Custom qa-docs logger use #1896

merged 15 commits into from
Sep 16, 2021

Conversation

roronoasins
Copy link

@roronoasins roronoasins commented Sep 15, 2021

Related issue
Closes #1879

Description

qa-docs was using the logging module, so we are using a custom qa-docs logger now.

All logging is filtered by level and showed using the standard output and a log file created in qa_docs_installation/log/todays_date-qa-docs.log.

Also, exceptions, errors, and raises are using custom loggers.

Logs example

A few of them can be found in #1879.

Log file creation:

luisgr@luisgr-pc:~/work/docgen/lib/python3.8/site-packages/wazuh_testing-4.3.0-py3.8.egg/wazuh_testing/qa_docs$ ls log/2021-09-15-12\:48\:27-qa-docs.log 
log/2021-09-15-12:48:27-qa-docs.log

@roronoasins roronoasins requested a review from mdengra September 15, 2021 10:51
@roronoasins roronoasins self-assigned this Sep 15, 2021
@mdengra mdengra linked an issue Sep 15, 2021 that may be closed by this pull request
11 tasks
@mdengra mdengra changed the title Custom qa-docs logger use Custom qa-docs logger use Sep 15, 2021

try:
DocGenerator.LOGGER.debug(f"Writing {doc_path}.json")
with open(doc_path + ".json", "w+") as outfile:
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps out_file would be better (improve readability to separate the words that form the variable name with underscores).

Suggested change
with open(doc_path + ".json", "w+") as outfile:
with open(doc_path + ".json", "w+") as out_file:

Copy link
Author

Choose a reason for hiding this comment

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

Done in: 86e5045

for regex in self.ignore_regex:
if regex.match(file):
DocGenerator.LOGGER.warning(f"File validation: {regex} not matchin with {file}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
DocGenerator.LOGGER.warning(f"File validation: {regex} not matchin with {file}")
DocGenerator.LOGGER.warning(f"File validation: {regex} not matching with {file}")

Copy link
Author

Choose a reason for hiding this comment

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

Done in: 86e5045

for regex in self.include_regex:
if regex.match(file):
DocGenerator.LOGGER.warning(f"File validation: {regex} not matchin with {file}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
DocGenerator.LOGGER.warning(f"File validation: {regex} not matchin with {file}")
DocGenerator.LOGGER.warning(f"File validation: {regex} not matching with {file}")

Copy link
Author

Choose a reason for hiding this comment

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

Done in: 86e5045

@@ -44,7 +46,9 @@ def is_valid_folder(self, path):
"""
for regex in self.ignore_regex:
if regex.match(path):
DocGenerator.LOGGER.warning(f"Folder validation: {regex} not matchin with {path}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
DocGenerator.LOGGER.warning(f"Folder validation: {regex} not matchin with {path}")
DocGenerator.LOGGER.warning(f"Folder validation: {regex} not matching with {path}")

Copy link
Author

Choose a reason for hiding this comment

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

Done in: 86e5045


try:
DocGenerator.LOGGER.debug(f"Writing {doc_path}.yaml")
with open(doc_path + ".yaml", "w+") as outfile:
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here.

Copy link
Author

Choose a reason for hiding this comment

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

Done in: 86e5045

Error: {inst}", stacklevel=2)
logging.warning(f"Failed to parse comment of function '{function.name}'' from module {self.scan_file}. \
Error: {inst}")
CodeParser.LOGGER.warning(f"Failed to parse comment of function {function.name} "
Copy link
Contributor

Choose a reason for hiding this comment

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

As the comment that is parsed is the documentation, how about this:

Suggested change
CodeParser.LOGGER.warning(f"Failed to parse comment of function {function.name} "
CodeParser.LOGGER.warning(f"Failed to parse test documentation in {function.name} "

Copy link
Author

Choose a reason for hiding this comment

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

Done in: 86e5045

else:
warnings.warn(f"Failed to parse comment of module {self.scan_file}. Error: {inst}", stacklevel=2)
logging.warning(f"Failed to parse comment of module {self.scan_file}. Error: {inst}")
CodeParser.LOGGER.warning(f"Failed to parse comment of module {self.scan_file}. Error: {inst}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
CodeParser.LOGGER.warning(f"Failed to parse comment of module {self.scan_file}. Error: {inst}")
CodeParser.LOGGER.warning(f"Failed to parse module documentation in {self.scan_file}. Error: {inst}")

Copy link
Author

Choose a reason for hiding this comment

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

Done in: 86e5045

"""
brief: Reads from the config file the regexes used to identify test files.
"""
Config.LOGGER.debug('Reading the regular expressions to include files from config file')
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Config.LOGGER.debug('Reading the regular expressions to include files from config file')
Config.LOGGER.debug('Reading the regular expressions from the config file to include test files')

Copy link
Author

Choose a reason for hiding this comment

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

Done in: 86e5045

if not 'Group files' in self._config_data:
logging.error("Config group files is empty")
raise Exception("Config group files is empty")
raise QAValueError('Config include paths are empty', Config.LOGGER.error)
Copy link
Contributor

@mdengra mdengra Sep 15, 2021

Choose a reason for hiding this comment

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

Suggested change
raise QAValueError('Config include paths are empty', Config.LOGGER.error)
raise QAValueError('The include paths of the configuration file are empty', Config.LOGGER.error)

Copy link
Author

Choose a reason for hiding this comment

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

Done in: 86e5045

"""
brief: Reads from the config file the regexes used to identify a test method.
"""
Config.LOGGER.debug('Reading the regular expressions to include test methods from config file')
Copy link
Contributor

@mdengra mdengra Sep 15, 2021

Choose a reason for hiding this comment

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

Suggested change
Config.LOGGER.debug('Reading the regular expressions to include test methods from config file')
Config.LOGGER.debug('Reading the regular expressions to include test methods from the config file')

Copy link
Author

Choose a reason for hiding this comment

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

Done in: 86e5045

"""
brief: Reads from the config file all the paths to be excluded from the parsing.
"""
Config.LOGGER.debug('Reading the paths to be ignored from config file')
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Config.LOGGER.debug('Reading the paths to be ignored from config file')
Config.LOGGER.debug('Reading the paths to be ignored from the config file')

Copy link
Author

Choose a reason for hiding this comment

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

Done in: 86e5045

if not 'Module' in self._config_data['Output fields']:
logging.error("Config output module fields is missing")
raise Exception("Config output module fields is missing")
raise QAValueError('Config output module fields is missing', Config.LOGGER.error)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
raise QAValueError('Config output module fields is missing', Config.LOGGER.error)
raise QAValueError('Config output module fields are missing', Config.LOGGER.error)

Copy link
Author

Choose a reason for hiding this comment

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

Done in: 86e5045

"""
brief: Reads from the config file the optional and mandatory fields for the test functions.
"""
Config.LOGGER.debug('Reading mandatory and optional test fields from config file')
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Config.LOGGER.debug('Reading mandatory and optional test fields from config file')
Config.LOGGER.debug('Reading mandatory and optional test fields from the config file')

Copy link
Author

Choose a reason for hiding this comment

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

Done in: 86e5045

if not 'Test' in self._config_data['Output fields']:
logging.error("Config output test fields is missing")
raise Exception("Config output test fields is missing")
raise QAValueError('Config output test fields is missing', Config.LOGGER.error)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
raise QAValueError('Config output test fields is missing', Config.LOGGER.error)
raise QAValueError('Config output test fields are missing', Config.LOGGER.error)

Copy link
Author

Choose a reason for hiding this comment

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

Done in: 86e5045

raise Exception("Config output fields is missing")
self._read_module_fields()
self._read_test_fields()
raise QAValueError('Config output fields is missing', Config.LOGGER.error)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
raise QAValueError('Config output fields is missing', Config.LOGGER.error)
raise QAValueError('Config output fields are missing', Config.LOGGER.error)

Copy link
Author

Choose a reason for hiding this comment

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

Done in: 86e5045

"""
brief: Reads from the configuration file the key to identify a Test Case list.
"""
Config.LOGGER.debug('Reading Test Case key from config file')
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Config.LOGGER.debug('Reading Test Case key from config file')
Config.LOGGER.debug('Reading Test Case key from the config file')

Copy link
Author

Choose a reason for hiding this comment

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

Done in: 86e5045

Test info:
- test_wazuh_min_version: wazuh_min_version
"""
Config.LOGGER.debug('Reading test info from config file')
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Config.LOGGER.debug('Reading test info from config file')
Config.LOGGER.debug('Reading test info from the config file')

Copy link
Author

Choose a reason for hiding this comment

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

Done in: 86e5045

"""
brief: Reads from the config file the optional and mandatory fields for the test module.
"""
Config.LOGGER.debug('Reading mandatory and optional module fields from config file')
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Config.LOGGER.debug('Reading mandatory and optional module fields from config file')
Config.LOGGER.debug('Reading mandatory and optional module fields from the config file')

Copy link
Author

Choose a reason for hiding this comment

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

Done in: 86e5045

"""
brief: Reads from the config file the file name to be identified with a group.
"""
Config.LOGGER.debug('Reading group files from config file')
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Config.LOGGER.debug('Reading group files from config file')
Config.LOGGER.debug('Reading group files from the config file')

Copy link
Author

Choose a reason for hiding this comment

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

Done in: 86e5045

"""
brief: Reads from the config file all the paths to be included in the parsing.
"""
Config.LOGGER.debug('Reading include paths from config file')
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Config.LOGGER.debug('Reading include paths from config file')
Config.LOGGER.debug('Reading include paths from the config file')

Copy link
Author

Choose a reason for hiding this comment

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

Done in: 86e5045

"""
if 'Project path' in self._config_data:
self.project_path = self._config_data['Project path']
Config.LOGGER.debug('Reading module info from config file')
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Config.LOGGER.debug('Reading module info from config file')
Config.LOGGER.debug('Reading module info from the config file')

Copy link
Author

Choose a reason for hiding this comment

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

Done in: 86e5045

@@ -38,7 +43,7 @@ def collect_test_cases(self, path):
- "path (string): Path of the test file to extract the test cases.
returns: "dictionary: The output of pytest parsed into a dictionary"
"""
logging.debug(f"Running pytest to collect testcases for '{path}'")
PytestWrap.LOGGER.debug(f"Running pytest to collect testcases for '{path}'")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
PytestWrap.LOGGER.debug(f"Running pytest to collect testcases for '{path}'")
PytestWrap.LOGGER.debug(f"Running pytest to collect test cases for '{path}'")

@@ -55,7 +59,7 @@ def validate_fields(self, required_fields, available_fields):
for field in required_fields:
if not check_existance(available_fields, field):
self.add_report(f"Mandatory field '{field}' is missing in file {self.scan_file}")
logging.error(f"Mandatory field '{field}' is missing in file {self.scan_file}")
Sanity.LOGGER.error(f"Mandatory field '{field}' is missing in file {self.scan_file}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Sanity.LOGGER.error(f"Mandatory field '{field}' is missing in file {self.scan_file}")
Sanity.LOGGER.error(f"Mandatory field '{field}' is missing in the file {self.scan_file}")

Copy link
Author

Choose a reason for hiding this comment

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

Done in: 9e21347

@@ -65,7 +69,7 @@ def validate_fields(self, required_fields, available_fields):
else:
if not check_existance(available_fields, field):
self.add_report(f"Mandatory field '{field}' is missing in file {self.scan_file}")
logging.error(f"Mandatory field '{field}' is missing in file {self.scan_file}")
Sanity.LOGGER.error(f"Mandatory field '{field}' is missing in file {self.scan_file}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Sanity.LOGGER.error(f"Mandatory field '{field}' is missing in file {self.scan_file}")
Sanity.LOGGER.error(f"Mandatory field '{field}' is missing in the file {self.scan_file}")

Copy link
Author

@roronoasins roronoasins Sep 16, 2021

Choose a reason for hiding this comment

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

Done in: 9e21347

Copy link
Contributor

@mdengra mdengra left a comment

Choose a reason for hiding this comment

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

Great job! Just a few suggestions.

@roronoasins roronoasins requested a review from mdengra September 16, 2021 07:35
Copy link
Contributor

@mdengra mdengra left a comment

Choose a reason for hiding this comment

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

Great job!

@roronoasins roronoasins merged commit 6a3a919 into 1864-qa-docs-fixes Sep 16, 2021
@roronoasins roronoasins deleted the 1879-qadocs-logging branch September 16, 2021 08:13
@roronoasins roronoasins linked an issue Sep 20, 2021 that may be closed by this pull request
32 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

qa-docs: Migrate logger and improve logging qa-docs: Refactoring and enhancement
2 participants