Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Expands the plugin’s pytest suite to cover the Atomic plugin’s service layer, GUI wiring, PowerShell parser behavior, and hook module behavior, while adding shared fixtures/stubs to run tests without a full Caldera runtime.
Changes:
- Added comprehensive unit tests for
AtomicService,AtomicGUI,hook.enable(), and the PowerShell parser. - Introduced
tests/conftest.pyto provide shared fixtures and Caldera dependency stubs. - Added
pytest.inito standardize pytest discovery and asyncio behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_hook.py | Adds async tests for hook.enable() and module attribute assertions |
| tests/test_atomic_svc.py | Adds broad unit coverage for AtomicService helpers and ingestion flow |
| tests/test_atomic_powershell.py | Adds tests documenting current parser behavior (including known bug) |
| tests/test_atomic_gui.py | Adds basic initialization/inheritance tests for AtomicGUI |
| tests/conftest.py | Adds shared fixtures and stubs to decouple tests from Caldera |
| pytest.ini | Configures pytest discovery and asyncio mode |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/conftest.py
Outdated
Comment on lines
+160
to
+165
| def generate_dummy_payload(): | ||
| with open(DUMMY_PAYLOAD_PATH, 'w') as f: | ||
| f.write(DUMMY_PAYLOAD_CONTENT) | ||
| yield DUMMY_PAYLOAD_PATH | ||
| if os.path.exists(DUMMY_PAYLOAD_PATH): | ||
| os.remove(DUMMY_PAYLOAD_PATH) |
Comment on lines
+112
to
119
| def test_handle_attachment(self, atomic_svc, generate_dummy_payload, tmp_path): | ||
| atomic_svc.payloads_dir = str(tmp_path / 'payloads') | ||
| os.makedirs(atomic_svc.payloads_dir, exist_ok=True) | ||
| target_hash = hashlib.md5(DUMMY_PAYLOAD_CONTENT.encode()).hexdigest() | ||
| target_name = target_hash[:PREFIX_HASH_LENGTH] + '_dummyatomicpayload' | ||
| target_path = atomic_svc.payloads_dir + '/' + target_name | ||
| target_path = os.path.join(atomic_svc.payloads_dir, target_name) | ||
| assert atomic_svc._handle_attachment(DUMMY_PAYLOAD_PATH) == target_name | ||
| assert os.path.isfile(target_path) |
Comment on lines
+6
to
+33
| class TestParserCheckedFlags: | ||
| """ | ||
| Tests for the Parser.checked_flags class attribute. | ||
|
|
||
| KNOWN BUG: `list('FullyQualifiedErrorId')` produces a list of individual | ||
| characters ['F', 'u', 'l', 'l', 'y', ...] instead of the intended | ||
| ['FullyQualifiedErrorId']. This means the parser checks for single | ||
| characters rather than the full error string. | ||
| """ | ||
|
|
||
| def test_checked_flags_is_list(self): | ||
| assert isinstance(Parser.checked_flags, list) | ||
|
|
||
| def test_checked_flags_known_bug_individual_characters(self): | ||
| """ | ||
| Demonstrates the known bug: list('FullyQualifiedErrorId') splits the | ||
| string into individual characters instead of wrapping it in a list. | ||
| """ | ||
| expected_buggy = list('FullyQualifiedErrorId') | ||
| assert Parser.checked_flags == expected_buggy | ||
| # This is what it SHOULD be: | ||
| expected_correct = ['FullyQualifiedErrorId'] | ||
| assert Parser.checked_flags != expected_correct | ||
|
|
||
| def test_checked_flags_length_is_wrong(self): | ||
| """The list has 21 entries (one per char) instead of 1.""" | ||
| assert len(Parser.checked_flags) == len('FullyQualifiedErrorId') | ||
| assert len(Parser.checked_flags) != 1 |
tests/test_hook.py
Outdated
Comment on lines
+49
to
+51
| patch('os.listdir', return_value=['abilities', 'other']): | ||
| await hook.enable(services) | ||
| # If abilities already exists, no cloning or populating should happen |
tests/test_hook.py
Outdated
Comment on lines
+120
to
+124
| with patch.object(hook, 'data_dir', '/tmp/atomic_test_hook_data'), \ | ||
| patch('os.listdir', return_value=['abilities']), \ | ||
| patch('hook.AtomicGUI'): | ||
| await hook.enable(services) | ||
| _ = mock_app_svc.application # verify it was accessed |
Contributor
Author
|
@copilot review |
|
@deacon-mp I've opened a new pull request, #57, to work on those changes. Once the pull request is ready, I'll request review from you. |
- conftest.py: generate_dummy_payload uses tmp_path fixture instead of hard-coded /tmp path; cleanup is now handled automatically by pytest - test_atomic_svc.py: attachment tests use fixture-returned path instead of the module-level DUMMY_PAYLOAD_PATH constant - test_atomic_powershell.py: mark checked_flags tests as xfail since they assert current buggy behavior (list() splitting string into chars) and will break once the underlying bug is fixed - test_hook.py: test_enable_creates_gui now patches hook.AtomicGUI and asserts it was called with expected args; test_enable_accesses_app uses PropertyMock for mock_app_svc.application and asserts it was accessed
deacon-mp
force-pushed
the
test/exhaustive-pytest-coverage
branch
from
74e5e0f to
9a2085c
Compare
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
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.
Summary
Modules covered
app/atomic_svc.py— 100+ tests covering all public/private methodsapp/atomic_gui.py— initialization and service wiring testsapp/parsers/atomic_powershell.py— parser logic and known bug documentationhook.py— module attributes and enable() functionTest plan
pytest tests/ -vto verify all tests passGenerated with Claude Code