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

Add tests for invalid config of github and office365 modules #1460

Merged
merged 5 commits into from
Jun 24, 2021

Conversation

TomasTurina
Copy link
Member

@TomasTurina TomasTurina commented Jun 16, 2021

Related issue
wazuh/wazuh#4983

Description

This PR includes:

  • Tests for github module configuration.
  • Tests for office365 module configuration.

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

@TomasTurina TomasTurina self-assigned this Jun 16, 2021
@spothound
Copy link
Contributor

spothound commented Jun 24, 2021

I'm reviewing this.

Wazuh branch Tested commit Revision
dev-office365-module d5925fc81c32829b5f153b1e020386e14a1c8ac9 0.commitd5925fc

Testing

test_github

Centos

image

Ubuntu

image

test_office

Centos

image

Ubuntu

image

Review notes

Copy link
Contributor

@spothound spothound left a comment

Choose a reason for hiding this comment

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

Ok, @TomasTurina, the code seems perfect to me, I've just noticed some unused import that I would like to remove. Also, I see some duplicated code that we may merge to make it more maintainable and readable, just a suggestion.

Finally, I have detected that those tests require the wazuh_modules.debug=2 value set in internal options to work. We are deprecating the option to add internal options on Jenkins, currently, we're encouraging the team to use a fixture to automatically set the required internal options before running the tests, so I would like to ask you to add this fixture to remove the requirement of setting this option previous to the test.

You have an example on how to do this here:

image

Please have all of this in consideration and ask us for review again when you finish making changes to the branch.

Best regards.

deps/wazuh_testing/wazuh_testing/github.py Outdated Show resolved Hide resolved
deps/wazuh_testing/wazuh_testing/office365.py Outdated Show resolved Hide resolved
@spothound spothound self-requested a review June 24, 2021 13:24
@spothound
Copy link
Contributor

spothound commented Jun 24, 2021

The requested changes were applied by TomasTaurina and now I am going to launch automatic tests again to confirm that now everything (including the automatic internal option configuration) works as expected.

Wazuh branch Tested commit Revision
dev-office365-module d5925fc81c32829b5f153b1e020386e14a1c8ac9 0.commitd5925fc

Testing

test_github

https://devel.ci.wazuh.info/job/Test_integration/814/console

Centos

image

Ubuntu

image

test_office

https://devel.ci.wazuh.info/job/Test_integration/815/console

Centos

image

Ubuntu

image

Copy link
Contributor

@spothound spothound left a comment

Choose a reason for hiding this comment

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

Awesome job, thank you very much!

@vikman90 vikman90 merged commit e1c8704 into master Jun 24, 2021
@vikman90 vikman90 deleted the dev-office365-module branch June 24, 2021 14:11
@snaow snaow mentioned this pull request Jan 27, 2022
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.

4 participants