Fix missing argument in test_macos_format_basic IT #1478

spothound · 2021-06-21T06:07:47Z

Related issue
Closes #1469

Description

It seems that format_macos_message_pattern' function arguments changed recently and were not updated in tests/integration/test_logcollector/test_macos/test_macos_format_basic.py leading to a logic failure because of a missing specification of the log type. This PR aims to fix that problem.

Configuration options

Tests

Proven that tests pass when they have to pass.
Proven that tests fail when they have to fail.
Python codebase satisfies PEP-8 style style guide. pycodestyle --max-line-length=120 --show-source --show-pep8 file.py.
Python codebase is documented following the Google Style for Python docstrings.
The test is documented in wazuh-qa/docs.
provision_documentation.sh generate the docs without errors.

snaow · 2021-06-22T08:03:54Z

tests/integration/test_logcollector/test_macos/test_macos_format_basic.py

@@ -45,7 +42,7 @@ def get_connection_configuration():
 return logcollector.DEFAULT_AUTHD_REMOTED_SIMULATOR_CONFIGURATION


-@pytest.mark.parametrize('macos_message', macos_log_messages)
+@pytest.mark.parametrize('macos_message', macos_log_messages, ids=[x['id'] for x in macos_log_messages])


x["id"] don't use letters for arrays, use descriptive names, for examples: "id_list"

Sure, but actually the array is called macos_log_message. In this case the x refers to each element of this array. Each element has a map, that's why we use x['id], but yes, I've renamed it to log_message as each element of the array is a log message.

98d147f

jmv74211 · 2021-06-22T09:05:42Z

deps/wazuh_testing/wazuh_testing/logcollector.py

@@ -760,4 +761,6 @@ def format_macos_message_pattern(process_name, message, type='log', subsystem=No
 elif type == 'activity':
 macos_message = f"{process_name}\[\d+\]: Created Activity ID.* Description: {message}"

+ assert macos_message is not None


It would be good to put a message indicating the reason when the assert fails

jmv74211 · 2021-06-22T09:12:48Z

tests/integration/test_logcollector/test_macos/test_macos_format_basic.py

@@ -45,7 +42,7 @@ def get_connection_configuration():
 return logcollector.DEFAULT_AUTHD_REMOTED_SIMULATOR_CONFIGURATION


-@pytest.mark.parametrize('macos_message', macos_log_messages)
+@pytest.mark.parametrize('macos_message', macos_log_messages, ids=[log_message['id'] for log_message in macos_log_messages])


Please, check PEP-8 guide styleE501 line too long (124 > 120 characters)

❓

jmv74211 · 2021-06-22T09:13:57Z

tests/integration/test_logcollector/test_macos/test_macos_format_basic.py

 expected_macos_message = logcollector.format_macos_message_pattern(
 'custom_log',
- logcollector.TEMPLATE_OSLOG_MESSAGE, macos_message['subsystem'],
+ logcollector.TEMPLATE_OSLOG_MESSAGE, 'log', macos_message['subsystem'],


Please, check PEP-8 guide style`

E501 line too long (127 > 120 characters)

Suggested change

logcollector.TEMPLATE_OSLOG_MESSAGE, 'log', macos_message['subsystem'],

logcollector.TEMPLATE_OSLOG_MESSAGE, subsystem=macos_message['subsystem'],

You can use the name of the argument to specify its value instead of putting the rest of the preceding arguments one by one. Python is flexible in this topic.

Also, in this case 'log' is the default value, so it is not necessary .

jmv74211 · 2021-06-22T09:18:35Z

tests/integration/test_logcollector/test_macos/test_macos_format_basic.py

 expected_macos_message = logcollector.format_macos_message_pattern(
 'custom_log',
- logcollector.TEMPLATE_OSLOG_MESSAGE, macos_message['subsystem'],
+ logcollector.TEMPLATE_OSLOG_MESSAGE, 'log', macos_message['subsystem'],


Suggested change

logcollector.TEMPLATE_OSLOG_MESSAGE, 'log', macos_message['subsystem'],

logcollector.TEMPLATE_OSLOG_MESSAGE, subsystem=macos_message['subsystem'],

jmv74211 · 2021-06-22T09:21:03Z

tests/integration/test_logcollector/test_macos/test_macos_format_basic.py

 expected_macos_message = logcollector.format_macos_message_pattern(
 'custom_log',
- logcollector.TEMPLATE_OSLOG_MESSAGE, macos_message['subsystem'],
+ logcollector.TEMPLATE_OSLOG_MESSAGE, 'log', macos_message['subsystem'],


You can use the name of the argument to specify its value instead of putting the rest of the preceding arguments one by one. Python is flexible in this topic.

Also, in this case 'log' is the default value, so it is not necessary .

spothound · 2021-06-22T09:45:35Z

deps/wazuh_testing/wazuh_testing/logcollector.py

@@ -479,6 +479,7 @@ def add_log_data(log_path, log_line_message, size_kib=1024, line_start=1, print_
 return line_start + lines - 1
 return 0

+


This is for pep8 requireements.

snaow · 2021-06-25T09:28:38Z

LGTM

spothound · 2021-06-25T10:26:27Z

ty

Fix missing argument in test_macos_format_basic IT

708738d

spothound requested a review from Rebits June 21, 2021 06:07

spothound self-assigned this Jun 21, 2021

spothound requested review from jmv74211 and snaow June 21, 2021 13:56

spothound mentioned this pull request Jun 22, 2021

Logcollector integration tests T0 macos: Test macOS log process. #1480

Closed

6 tasks

snaow reviewed Jun 22, 2021

View reviewed changes

snaow previously approved these changes Jun 22, 2021

View reviewed changes

Improve variable name in logcollector IT

98d147f

spothound dismissed snaow’s stale review via 98d147f June 22, 2021 08:14

jmv74211 suggested changes Jun 22, 2021

View reviewed changes

spothound added 2 commits June 22, 2021 11:34

Add assert msg to format_macos_message_pattern

ed0810c

Style changes to meet pep8 requirements

e506774

spothound commented Jun 22, 2021

View reviewed changes

spothound requested review from jmv74211 and snaow and removed request for snaow and Rebits June 22, 2021 15:10

jmv74211 approved these changes Jun 23, 2021

View reviewed changes

snaow approved these changes Jun 25, 2021

View reviewed changes

snaow merged commit d665ab6 into master Jun 25, 2021

snaow deleted the 1469-fix-macos-logcollector-it branch June 25, 2021 09:28

snaow mentioned this pull request Jan 27, 2022

QA Release - Rev 430031 #2500

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix missing argument in test_macos_format_basic IT #1478

Fix missing argument in test_macos_format_basic IT #1478

spothound commented Jun 21, 2021 •

edited

Loading

snaow Jun 22, 2021

spothound Jun 22, 2021

jmv74211 Jun 22, 2021

spothound Jun 22, 2021

jmv74211 Jun 22, 2021

spothound Jun 22, 2021

jmv74211 Jun 22, 2021

jmv74211 Jun 22, 2021

jmv74211 Jun 22, 2021

spothound Jun 22, 2021

jmv74211 Jun 22, 2021

jmv74211 Jun 22, 2021

spothound Jun 22, 2021

snaow commented Jun 25, 2021

spothound commented Jun 25, 2021

	logcollector.TEMPLATE_OSLOG_MESSAGE, 'log', macos_message['subsystem'],
	logcollector.TEMPLATE_OSLOG_MESSAGE, subsystem=macos_message['subsystem'],

		@@ -479,6 +479,7 @@ def add_log_data(log_path, log_line_message, size_kib=1024, line_start=1, print_
		return line_start + lines - 1
		return 0

Fix missing argument in test_macos_format_basic IT #1478

Fix missing argument in test_macos_format_basic IT #1478

Conversation

spothound commented Jun 21, 2021 • edited Loading

Description

Configuration options

Tests

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

snaow commented Jun 25, 2021

spothound commented Jun 25, 2021

spothound commented Jun 21, 2021 •

edited

Loading