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

FIM IT: Fix file_limit and registry_limit test suites #3381

Merged

Conversation

Deblintrake09
Copy link
Contributor

@Deblintrake09 Deblintrake09 commented Sep 29, 2022

Related issue
#3036

Description

This PR Aims to fix the tests related to the film database limit options file_limit and registry_limit. Also the test_max_eps test has been fixed.

Updated

  • Updated configs and callbacks for test_files/tests_file_limit test suite
  • Updated configs and callbacks for test_registry/test_registry_limit test suite
  • Fixed test_files/test_max_eps test suite flaky behavior

Testing performed

Tester Test path Jenkins Local OS Commit Notes
@Deblintrake09 test_fim 🟢🟢🟢 🟢 🟢 🟢 Manager 1852642
@Deblintrake09 test_fim 🔴 🔴 🔴 ⚫⚫⚫ Windows 1852642 Fails are unrelated to development. Bug reported in wazuh/wazuh#14956 and wazuh/wazuh#14957
@Deblintrake09 test_fim 🟢 🟢 🟢 ⚫⚫⚫ Linux Agent 1852642
@Deblintrake09 test_fim 🟢 🟢 🟢 ⚫⚫⚫ Solaris 1852642
@Deblintrake09 test_fim 🟢 🟢 🟢 ⚫⚫⚫ macOS 1852642
@user (Reviewer) ⚫⚫⚫ 🚫 🚫 🚫 Nothing to highlight

Conclusion

It is recommended that this branch is merged into branch https://github.com/wazuh/wazuh-qa/tree/1915-fim-rsync-new-synchronization-mechanism, and the errors caused by wazuh/wazuh#14956 and wazuh/wazuh#14957, are handled in that branch, since those errors are in tests not related to this Issue.

Copy link
Member

@damarisg damarisg left a comment

Choose a reason for hiding this comment

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

Also, I would like that you update the description of the PR because I can see another case modified that isn't from the test_files/tests_file_limit test suite that you mentioned.

deps/wazuh_testing/wazuh_testing/modules/fim/__init__.py Outdated Show resolved Hide resolved
@@ -171,7 +171,7 @@ def test_max_eps(configure_local_internal_options_module, get_configuration, con
error_message=ERR_MSG_MONITORING_PATH).result()
create_multiple_files(get_configuration)
# Create files to read max_eps files with added events
n_results = max_eps + 5
n_results = max_eps * 2
Copy link
Member

Choose a reason for hiding this comment

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

Could you explain the reason to change by *2, please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was changed because the max_eps+5 was the number of files that would be created, and then the number of results the FileMonitor is trying to get. The main issue was that this that later the results are filtered and it is checked if the number that happened a specific second matches the max_eps. having just 5 additional events is too small of a margin because the first/last 6 events could happen in another second (before or after) and that caused the event test to fail. By doubling the ammount of files/events generated, Now the having less than the expected events does not happen.

CHANGELOG.md Outdated Show resolved Hide resolved
@damarisg
Copy link
Member

Currently, there are problems with the core branch, so the core team is working on fixing them.

After checking the changes applied, and the executions launched by the developer. This PR is approved and merged with the intermediate branch.

@damarisg damarisg merged commit d528ba6 into 1915-fim-rsync-new-synchronization-mechanism Oct 26, 2022
@damarisg damarisg deleted the 3036-fimdb-limits branch October 26, 2022 17:56
nico-stefani added a commit that referenced this pull request Jan 31, 2023
nico-stefani added a commit that referenced this pull request Feb 1, 2023
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.

Tests development - Changes in FIMDB limits configuration
2 participants