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

Create daemon handler fixture for integration test #1826

Merged
merged 14 commits into from
Sep 7, 2021

Conversation

Rebits
Copy link
Member

@Rebits Rebits commented Sep 1, 2021

Related issue
#1825

Description

This PR closes #1825. It adds a common Wazuh daemon handler fixture for all integration tests, according to the repository standard

Usage

In order to use this fixture, the test should define the following dictionary at the beginning of the module:

daemons_handler_configuration = {'daemons': ['wazuh-remoted'], 'ignore_errors': True}

In case of the required of restart all wazuh, it is possible to use wazuh_control using the following configuration

daemons_handler_configuration = {'wazuh_control': True, 'ignore_errors': True}

Where:

  • daemons (list): List of daemons to use. If empty, all wazuh will be used.
  • wazuh_control (boolean): Enable restart of all wazuh services using wazuh_control.
  • ignore_errors (boolean): Enable ignore errors in daemon handling process.

Tests

  • Proven that this fixture works correctly
  • 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.

@Rebits Rebits self-assigned this Sep 1, 2021
@Selutario
Copy link
Contributor

Additional info

In the case of creating a variable with the necessary daemons for the API to work, the list is as follows:

  • wazuh-apid
  • wazuh-modulesd
  • wazuh-analysisd
  • wazuh-execd
  • wazuh-db
  • wazuh-remoted

If any of those daemons are not ready, the error below will be returned for any API request:

{
  "title": "Bad Request",
  "detail": "Some Wazuh daemons are not ready yet in node \"master-node\" (wazuh-modulesd->stopped, wazuh-analysisd->stopped, wazuh-execd->stopped, wazuh-remoted->stopped)",
  "dapi_errors": {
    "master-node": {
      "error": "Some Wazuh daemons are not ready yet in node \"master-node\" (wazuh-modulesd->stopped, wazuh-analysisd->stopped, wazuh-execd->stopped, wazuh-remoted->stopped)"
    }
  },
  "error": 1017
}

Regards.

Copy link
Contributor

@jmv74211 jmv74211 left a comment

Choose a reason for hiding this comment

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

Good job. Some changes are requested.

Note: I have observed that you put some debug logs. This fixture will be called in every test for each use case, so these debug logs can be very noisy. Take this into account.

deps/wazuh_testing/wazuh_testing/tools/__init__.py Outdated Show resolved Hide resolved
tests/integration/conftest.py Outdated Show resolved Hide resolved
tests/integration/conftest.py Outdated Show resolved Hide resolved
tests/integration/conftest.py Show resolved Hide resolved
tests/integration/conftest.py Show resolved Hide resolved
@Rebits
Copy link
Member Author

Rebits commented Sep 2, 2021

Respecting to this comment, yes, that could be an issue, but IMHO it is important to begin to include some verbose in our testing procedure. Maybe it would be necessary to create a new logs levels and implement tools to handle the log-cli-level, log-file-level and log-file of pytest.

Rename api daemons
Fix minnor docstring error in daemon_handler fixture
Improve daemons list valid value

Co-authored-by: Jonathan <jonathan.martin@wazuh.com>
juliamagan
juliamagan previously approved these changes Sep 6, 2021
Copy link
Member

@juliamagan juliamagan left a comment

Choose a reason for hiding this comment

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

LGTM

@juliamagan
Copy link
Member

We can test these changes before the merge in temporal branch: common-fixtures-testing

"""
daemons = []
ignore_errors = False
wazuh_control = False
Copy link
Contributor

@jmv74211 jmv74211 Sep 7, 2021

Choose a reason for hiding this comment

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

Can you change this for all_daemons or something similar. wazuh_control is a bit confusing

jmv74211
jmv74211 previously approved these changes Sep 7, 2021
@Rebits Rebits requested a review from juliamagan September 7, 2021 08:06
juliamagan
juliamagan previously approved these changes Sep 7, 2021
@Rebits Rebits dismissed stale reviews from juliamagan and jmv74211 via b02648a September 7, 2021 08:11
@jmv74211 jmv74211 merged commit f78f310 into master Sep 7, 2021
@jmv74211 jmv74211 deleted the 1825_daemon_handler_fixture branch September 7, 2021 08:12
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.

Wazuh daemons handler fixture
4 participants