macOS test added #5264
Merged
macOS test added #5264
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pro-akim
force-pushed
the
enhancement/5229-macOS-tests-added
branch
from
3a1f915
to
2c925fa
Compare
2 tasks
mhamra
suggested changes
Apr 25, 2024
| return data['platform'] | ||
| else: | ||
| raise KeyError("The 'platform' key was not found in the YAML file.") | ||
| except FileNotFoundError: |
There was a problem hiding this comment.
The exception handling block logs the exception but does not raise it again. If the exception handling does not raise the exception, the testing module will continue without returning an error exit code to the workflow_engine.
The workflow will not abort all the tasks if the returned exit code is 0.
There was a problem hiding this comment.
This should be fixed as an improvement in a future process where obtaining the os_type variable is obtained from a single call and not using constant calls to the disk file
| return data['arch'] | ||
| else: | ||
| raise KeyError("The 'platform' key was not found in the YAML file.") | ||
| except FileNotFoundError: |
There was a problem hiding this comment.
I make the same observation to this exception handling block.
There was a problem hiding this comment.
This should be fixed as an improvement in a future process where obtaining the os_type variable is obtained from a single call and not using constant calls to the disk file
| @@ -71,8 +71,7 @@ def test_service(wazuh_params): | |||
| GeneralComponentActions.component_stop(agent_params, 'wazuh-agent') | |||
|
|
|||
| for agent_names, agent_params in wazuh_params['agents'].items(): | |||
| assert 'inactive' in GeneralComponentActions.get_component_status(agent_params, 'wazuh-agent'), logger.error(f'{agent_names} is still active by command') | |||
| assert not GeneralComponentActions.isComponentActive(agent_params, 'wazuh-agent'), logger.error(f'{agent_names} is still active by command') | |||
| assert 'inactive' in GeneralComponentActions.get_component_status(agent_params, 'wazuh-agent') or 'not running' in GeneralComponentActions.get_component_status(agent_params, 'wazuh-agent'), logger.error(f'{agent_names} is still active by command') | |||
There was a problem hiding this comment.
I don't recommend to call logger.error in assert statements. The second argument is the message assigned to the AssertionError exception. If you use logger.error, the return will be None, which will be the exception's message. Here's an example of what is the problem:
>>> import logging
>>> logger = logging.getLogger()
>>> assert False, logger.error('Error Message')
Error Message
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
AssertionError: None
>>> assert False, 'Error Message'
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
AssertionError: Error Message
Is it required to log the error? Pytest shows all the failures in the output report.
There was a problem hiding this comment.
The correctness of all asserts can be evaluated as a proposal for improvement since all test assertions are developed with that structure.
TESTSI've run the test In the first run, the workflow file successfully allocated and provisioned the manager but failed to allocate the macOS agent. After investigating the issue, I've found that the allocator modules require the installation of the The macOS agent was allocated ok, and the testing task passed without failures. workflow.logI've run the test All the agent tests passed with no failures. |
mhamra
approved these changes
Apr 25, 2024
There was a problem hiding this comment.
After a meeting with @pro-akim, we discussed the requested changes and arrived at the following decisions:
-
Exception blocks in
deployability/modules/testing/tests/helpers/generic.py
Many tests are using theHostInformation.get_os_typemethod. The current implementation reads the track file. A better solution is to open the YAML file, convert it into a dictionary or an object, and reimplement the function to access the object instead of the file. We'll create a new issue to implement this procedure. -
assertions and log
- We will replace the calls to
logger.errorwith a string message in all theassertstatements.
We must define whether to send an error message to the log when the assert conditions are false. Pytest tests send the errors to the final report; the test doesn't need to duplicate the report. - We will create a new enhancement-type issue to implement the above bullets.
- We will replace the calls to
pro-akim
commented
Apr 26, 2024
| @@ -71,8 +71,7 @@ def test_service(wazuh_params): | |||
| GeneralComponentActions.component_stop(agent_params, 'wazuh-agent') | |||
|
|
|||
| for agent_names, agent_params in wazuh_params['agents'].items(): | |||
| assert 'inactive' in GeneralComponentActions.get_component_status(agent_params, 'wazuh-agent'), logger.error(f'{agent_names} is still active by command') | |||
| assert not GeneralComponentActions.isComponentActive(agent_params, 'wazuh-agent'), logger.error(f'{agent_names} is still active by command') | |||
| assert 'inactive' in GeneralComponentActions.get_component_status(agent_params, 'wazuh-agent') or 'not running' in GeneralComponentActions.get_component_status(agent_params, 'wazuh-agent'), logger.error(f'{agent_names} is still active by command') | |||
There was a problem hiding this comment.
The correctness of all asserts can be evaluated as a proposal for improvement since all test assertions are developed with that structure.
| return data['arch'] | ||
| else: | ||
| raise KeyError("The 'platform' key was not found in the YAML file.") | ||
| except FileNotFoundError: |
There was a problem hiding this comment.
This should be fixed as an improvement in a future process where obtaining the os_type variable is obtained from a single call and not using constant calls to the disk file
| return data['platform'] | ||
| else: | ||
| raise KeyError("The 'platform' key was not found in the YAML file.") | ||
| except FileNotFoundError: |
There was a problem hiding this comment.
This should be fixed as an improvement in a future process where obtaining the os_type variable is obtained from a single call and not using constant calls to the disk file
rauldpm
requested changes
Apr 26, 2024
deployability/modules/workflow_engine/examples/agent/aws/test-agent-complete-macOs.yaml
Show resolved
Hide resolved
deployability/modules/workflow_engine/examples/agent/vagrant/test-agent-complete-macOs.yaml
Show resolved
Hide resolved
rauldpm
approved these changes
Apr 26, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adding features for macOs agent testing