Speed up unit tests #2626
Merged
Speed up unit tests #2626
Conversation
This file contains hidden or 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
brokensound77
requested review from
Mikaayenson and
terrancedejesus
as code owners
terrancedejesus
approved these changes
Mar 7, 2023
Contributor
terrancedejesus
left a comment
terrancedejesus
left a comment
There was a problem hiding this comment.
LGTM. From my understanding, there is two enhancements here. The first, we use the @lru_cache decorator for the rule loader, which loads all rules. This way all rules are not loaded multiple times as it is already cached. Second, the setUp method takes the value of RULE_LOADER_FAIL which is set by setUpClass and if True stops the rule loader from doing additional runs.
@brokensound77 please correct my understanding if it is incorrect.
Contributor
Author
yep, exactly. Any class that inherits the |
Mikaayenson
reviewed
Mar 7, 2023
Contributor
Mikaayenson
left a comment
Mikaayenson
left a comment
There was a problem hiding this comment.
1.✔️ Ran the unit test a couple ways to verify timing with pytest
(detection_dev) ➜ detection-rules git:(main) ✗ time pytest
============================================================================================================================================================================================================================= test session starts ==============================================================================================================================================================================================================================
platform darwin -- Python 3.10.9, pytest-7.1.2, pluggy-1.0.0
rootdir: /Users/stryker/workspace/Elastic/detection-rules, configfile: pyproject.toml
plugins: typeguard-2.13.3, profiling-1.7.0
collected 118 items
tests/test_all_rules.py ..................s........... [ 25%]
tests/test_gh_workflows.py . [ 26%]
tests/test_mappings.py ... [ 28%]
tests/test_packages.py ..... [ 33%]
tests/test_schemas.py .......... [ 41%]
tests/test_toml_formatter.py ... [ 44%]
tests/test_utils.py .... [ 47%]
tests/test_version_locking.py . [ 48%]
tests/kuery/test_dsl.py ....... [ 54%]
tests/kuery/test_eql2kql.py ......... [ 61%]
tests/kuery/test_evaluator.py ............. [ 72%]
tests/kuery/test_kql2eql.py .......... [ 81%]
tests/kuery/test_lint.py ............ [ 91%]
tests/kuery/test_parser.py .......... [100%]
======================================================================================================================================================================================================================= 117 passed, 1 skipped in 55.38s ========================================================================================================================================================================================================================
pytest 42.96s user 3.50s system 81% cpu 56.673 total
(detection_dev) ➜ detection-rules git:(main) ✗ time pytest
============================================================================================================================================================================================================================= test session starts ==============================================================================================================================================================================================================================
platform darwin -- Python 3.10.9, pytest-7.1.2, pluggy-1.0.0
rootdir: /Users/stryker/workspace/Elastic/detection-rules, configfile: pyproject.toml
plugins: typeguard-2.13.3, profiling-1.7.0
collected 118 items
tests/test_all_rules.py Fsssssssssssssssssssssssssssss [ 25%]
tests/test_gh_workflows.py . [ 26%]
tests/test_mappings.py ss. [ 28%]
tests/test_packages.py ssss. [ 33%]
tests/test_schemas.py .......... [ 41%]
tests/test_toml_formatter.py ... [ 44%]
tests/test_utils.py .... [ 47%]
tests/test_version_locking.py . [ 48%]
tests/kuery/test_dsl.py ....... [ 54%]
tests/kuery/test_eql2kql.py ......... [ 61%]
tests/kuery/test_evaluator.py ............. [ 72%]
tests/kuery/test_kql2eql.py .......... [ 81%]
tests/kuery/test_lint.py ............ [ 91%]
tests/kuery/test_parser.py .......... [100%]
=================================================================================================================================================================================================================================== FAILURES ===================================================================================================================================================================================================================================
________________________________________________________________________________________________________________________________________________________________________________________________________________ TestValidRules.test_all_rule_queries_optimized ________________________________________________________________________________________________________________________________________________________________________________________________________________
tests/base.py:60: in setUp
self.fail(f'Rule loader failure: \n{RULE_LOADER_FAIL_MSG}')
E AssertionError: Rule loader failure:
E Error at line:6,column:10
E Invalid syntax
E [network where host.os.type == "windows" and process.name : "MSBuild.exe" and and
E not (destination.ip == "127.0.0.1" and source.ip == "127.0.0.1")]
E ^
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- Captured stdout setup -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Error loading rule in /Users/stryker/workspace/Elastic/detection-rules/rules/windows/defense_evasion_msbuild_beacon_sequence.toml
=========================================================================================================================================================================================================================== short test summary info ============================================================================================================================================================================================================================
FAILED tests/test_all_rules.py::TestValidRules::test_all_rule_queries_optimized - AssertionError: Rule loader failure:
================================================================================================================================================================================================================== 1 failed, 82 passed, 35 skipped in 29.28s ===================================================================================================================================================================================================================
pytest 22.43s user 1.94s system 80% cpu 30.400 total
(detection_dev) ➜ detection-rules git:(main) ✗ time pytest
============================================================================================================================================================================================================================= test session starts ==============================================================================================================================================================================================================================
platform darwin -- Python 3.10.9, pytest-7.1.2, pluggy-1.0.0
rootdir: /Users/stryker/workspace/Elastic/detection-rules, configfile: pyproject.toml
plugins: typeguard-2.13.3, profiling-1.7.0
collected 118 items
tests/test_all_rules.py Fsssssssssssssssssssssssssssss [ 25%]
tests/test_gh_workflows.py . [ 26%]
tests/test_mappings.py ss. [ 28%]
tests/test_packages.py ssss. [ 33%]
tests/test_schemas.py .......... [ 41%]
tests/test_toml_formatter.py ... [ 44%]
tests/test_utils.py .... [ 47%]
tests/test_version_locking.py . [ 48%]
tests/kuery/test_dsl.py ....... [ 54%]
tests/kuery/test_eql2kql.py ......... [ 61%]
tests/kuery/test_evaluator.py ............. [ 72%]
tests/kuery/test_kql2eql.py .......... [ 81%]
tests/kuery/test_lint.py ............ [ 91%]
tests/kuery/test_parser.py .......... [100%]
=================================================================================================================================================================================================================================== FAILURES ===================================================================================================================================================================================================================================
________________________________________________________________________________________________________________________________________________________________________________________________________________ TestValidRules.test_all_rule_queries_optimized ________________________________________________________________________________________________________________________________________________________________________________________________________________
tests/base.py:60: in setUp
self.fail(f'Rule loader failure: \n{RULE_LOADER_FAIL_MSG}')
E AssertionError: Rule loader failure:
E {'rule': [ValidationError({'rule_id': ['Missing data for required field.']}), ValidationError({'threshold': ['Missing data for required field.'], 'language': ['Must be one of: kuery, lucene.'], 'type': ['Must be equal to threshold.'], 'rule_id': ['Missing data for required field.']}), ValidationError({'threat_index': ['Missing data for required field.'], 'language': ['Must be one of: kuery, lucene.'], 'type': ['Must be equal to threat_match.'], 'rule_id': ['Missing data for required field.'], 'threat_mapping': ['Missing data for required field.']}), ValidationError({'type': ['Must be equal to machine_learning.'], 'rule_id': ['Missing data for required field.'], 'anomaly_threshold': ['Missing data for required field.'], 'machine_learning_job_id': ['Missing data for required field.'], 'index': ['Unknown field.'], 'query': ['Unknown field.'], 'language': ['Unknown field.']}), ValidationError({'language': ['Must be one of: kuery, lucene.'], 'type': ['Must be equal to query.'], 'rule_id': ['Missing data for required field.']}), ValidationError({'language': ['Must be one of: kuery, lucene.'], 'type': ['Must be equal to new_terms.'], 'rule_id': ['Missing data for required field.'], 'new_terms': ['Missing data for required field.']})]}
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- Captured stdout setup -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Error loading rule in /Users/stryker/workspace/Elastic/detection-rules/rules/windows/defense_evasion_msbuild_beacon_sequence.toml
=========================================================================================================================================================================================================================== short test summary info ============================================================================================================================================================================================================================
FAILED tests/test_all_rules.py::TestValidRules::test_all_rule_queries_optimized - AssertionError: Rule loader failure:
================================================================================================================================================================================================================== 1 failed, 82 passed, 35 skipped in 28.81s ===================================================================================================================================================================================================================
pytest 22.57s user 2.15s system 82% cpu 29.846 total
(detection_dev) ➜ detection-rules git:(main) ✗ time pytest
============================================================================================================================================================================================================================= test session starts ==============================================================================================================================================================================================================================
platform darwin -- Python 3.10.9, pytest-7.1.2, pluggy-1.0.0
rootdir: /Users/stryker/workspace/Elastic/detection-rules, configfile: pyproject.toml
plugins: typeguard-2.13.3, profiling-1.7.0
collected 118 items
tests/test_all_rules.py ..................s........... [ 25%]
tests/test_gh_workflows.py . [ 26%]
tests/test_mappings.py ... [ 28%]
tests/test_packages.py ..... [ 33%]
tests/test_schemas.py .......... [ 41%]
tests/test_toml_formatter.py ... [ 44%]
tests/test_utils.py .... [ 47%]
tests/test_version_locking.py . [ 48%]
tests/kuery/test_dsl.py ....... [ 54%]
tests/kuery/test_eql2kql.py ......... [ 61%]
tests/kuery/test_evaluator.py ............. [ 72%]
tests/kuery/test_kql2eql.py .......... [ 81%]
tests/kuery/test_lint.py ............ [ 91%]
tests/kuery/test_parser.py .......... [100%]
======================================================================================================================================================================================================================= 117 passed, 1 skipped in 53.34s ========================================================================================================================================================================================================================
pytest 42.79s user 3.55s system 84% cpu 54.672 total
(detection_dev) ➜ detection-rules git:(main) ✗
- I left some cleanup suggestions.
Mikaayenson
approved these changes
Mar 7, 2023
protectionsmachine
pushed a commit
that referenced
this pull request
Mar 7, 2023
* cache rule loader; skip rule tests on RL failure ------- Co-authored-by: brokensound77 <brokensound77@users.noreply.github.com> (cherry picked from commit cd6a598)
protectionsmachine
pushed a commit
that referenced
this pull request
Mar 7, 2023
* cache rule loader; skip rule tests on RL failure ------- Co-authored-by: brokensound77 <brokensound77@users.noreply.github.com> (cherry picked from commit cd6a598)
protectionsmachine
pushed a commit
that referenced
this pull request
Mar 7, 2023
* cache rule loader; skip rule tests on RL failure ------- Co-authored-by: brokensound77 <brokensound77@users.noreply.github.com> (cherry picked from commit cd6a598)
protectionsmachine
pushed a commit
that referenced
this pull request
Mar 7, 2023
* cache rule loader; skip rule tests on RL failure ------- Co-authored-by: brokensound77 <brokensound77@users.noreply.github.com> (cherry picked from commit cd6a598)
protectionsmachine
pushed a commit
that referenced
this pull request
Mar 7, 2023
* cache rule loader; skip rule tests on RL failure ------- Co-authored-by: brokensound77 <brokensound77@users.noreply.github.com> (cherry picked from commit cd6a598)
protectionsmachine
pushed a commit
that referenced
this pull request
Mar 7, 2023
* cache rule loader; skip rule tests on RL failure ------- Co-authored-by: brokensound77 <brokensound77@users.noreply.github.com> (cherry picked from commit cd6a598)
protectionsmachine
pushed a commit
that referenced
this pull request
Mar 7, 2023
* cache rule loader; skip rule tests on RL failure ------- Co-authored-by: brokensound77 <brokensound77@users.noreply.github.com> (cherry picked from commit cd6a598)
protectionsmachine
pushed a commit
that referenced
this pull request
Mar 7, 2023
* cache rule loader; skip rule tests on RL failure ------- Co-authored-by: brokensound77 <brokensound77@users.noreply.github.com> (cherry picked from commit cd6a598)
protectionsmachine
pushed a commit
that referenced
this pull request
Mar 7, 2023
* cache rule loader; skip rule tests on RL failure ------- Co-authored-by: brokensound77 <brokensound77@users.noreply.github.com> (cherry picked from commit cd6a598)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Issues
resolves #1869
resolves #2623
Summary
This PR caches the rule loader default rules for unit tests. It also skips rule based-test when the rule loader fails.
Details
To test, change any rule
authortoauthorzand run unit testsDuration before:
Duration after:
Speeding up passing
Each unittest class in
test_all_rulesloads theBaseRuleTest.setUpmethod when setting up unit tests. This meant that the rule loader was run multiple times. It now caches that load to limit it to a single load.Speeding up failures
Now, if the loader fails, it will skip all rule based test dependent on the leader