Open
Conversation
IPv6 link-local addresses are automatically assigned but they are poor options for reliable connectivity in an IPv6 network. Filter out these addresses carefully. Signed-off-by: Major Hayden <major@redhat.com>
Add a makefile for quick local testing that includes most of what's being run in CI. 'make verify' runs everything. Signed-off-by: Major Hayden <major@redhat.com>
Add a global ssh timeout in the main configuration file, but also allow the project to override timeouts for tools that sometimes take a long time to run. The default is now 30 seconds. This avoids situations where the connection hangs indefinitely due to firewall rules or misconfigured networks. Signed-off-by: Major Hayden <major@redhat.com>
Make ssh key verification configurable, but leave it off for now until this project is ready for production. Signed-off-by: Major Hayden <major@redhat.com>
…tion (rhel-lightspeed#91) * refactor(tests): add imports and helper for test_logs.py refactor Add imports for pytest parameterization (nullcontext, AsyncMock, pytest) and execute_command for mock spec validation. Add assert_tool_result_structure() helper to extract repeated assertion pattern used across all test functions. Signed-off-by: Major Hayden <major@redhat.com> * refactor(tests): add pytest fixtures for common mocking patterns Add fixtures to replace inline mocking patterns: - mock_execute_command: patches with spec for type safety - mock_path_exists: patches os.path.exists - mock_allowed_log_paths: fixture factory for CONFIG mocking Signed-off-by: Major Hayden <major@redhat.com> * refactor(tests): consolidate TestGetJournalLogs with parameterization Convert 6 individual filter tests into 1 parameterized test function. Migrate to mock_execute_command fixture and assert_tool_result_structure helper for cleaner, DRY test code. Signed-off-by: Major Hayden <major@redhat.com> * refactor(tests): consolidate TestGetAuditLogs with parameterization Parameterize line count and error handling tests. Migrate to fixtures (mock_execute_command, mock_path_exists) and helper function. Signed-off-by: Major Hayden <major@redhat.com> * refactor(tests): consolidate TestReadLogFile with fixtures Add class-level setup_log_file fixture for test file creation. Parameterize line count, path validation, and error handling tests. Complete migration to fixtures and helper function. Signed-off-by: Major Hayden <major@redhat.com> --------- Signed-off-by: Major Hayden <major@redhat.com>
- Remind LLMs to run tests after they finish their work - Guide LLMs about fixtures and mocks preferences Signed-off-by: Major Hayden <major@redhat.com>
…-lightspeed#94) * refactor(tests): consolidate TestListBlockDevices with fixtures - Add mock_disk_io_stats and mock_partition fixtures - Add verify_result_structure helper for common assertions - Combine lsblk success tests into parameterized test - Combine psutil fallback tests into parameterized test - Reduce ~165 lines to ~90 lines (~45% reduction) - Maintain 100% test coverage with all 7 test scenarios Signed-off-by: Major Hayden <major@redhat.com> * docs(AGENTS): add mock spec guidelines Add guidelines for using autospec and spec with mocks to ensure mocks accurately reflect the real objects they replace. Signed-off-by: Major Hayden <major@redhat.com> --------- Signed-off-by: Major Hayden <major@redhat.com>
…e system (rhel-lightspeed#71) * Add a helper function for getting the pull path of an executable on a remote host This allows testing for whethere or not a command exists before the tool tries to use it and report a more useful failure to the LLM client. * Use get_bit_path with a few tools as an example * Create separate functions for local and remote * Remove username as a parameter Rather than a paremeter, this should come from config. The config value should default to None so that the ssh_config settings can be used. This may cause some problems with the ssh connection caching as it is now by host. * Move command lookup into execute_command This may be a bit too helpful and frustrate the caller in the future, but works well given the current state of the code. * Search sbin paths by default * Raise ToolError for all tools * Update tests * Provide a spec to avoid incorrect mocking and remove warning * Log ssh username using value from the connection This lets async ssh do the work of getting the username based on ssh_config which is more in line with user expectations. * Fixup tests * Append to PATH more succinctly * Separate ssh tests into separate modules * Fixup more tests More accurate and less verbose patching * Move command lookup into the local and remote execution functions This gets rid of another ‘if host’ branch in the code. Now the local and remote exeucution code takes care of looking up the command in the way that makes sense for that context. * Correct typeo in docs * Pass in unpacked args * Change execption raised when file is not found * Let exceptions bubble up to tool call The framework catches any raised execption in a tool and raises a ToolError. There is no need to explicitly raise a ToolError since any exception in a tool ends up being raised as a ToolError. * Improve error test cases for log tools * Let the exception bubble up * Consolidate tests * Simplify key discovery tests * Make mocks simpler and add additional test case * Add explicit tests for get_bin_path and get_remote_bin_path * Do not catch exception raised by get_remote_bin_path Let it bubble all the way up. * Bring some sanity to the connection manager tests * Fix typo
Allow CodeRabbit to comment on problems it finds in pull requests without being overly chatty or focusing on issues solved by existing tools, such as linters. Signed-off-by: Major Hayden <major@redhat.com>
narmaku
suggested changes
Dec 15, 2025
| enable-cache: true | ||
| prune-cache: false | ||
|
|
||
| - name: Run Installation and Verification |
There was a problem hiding this comment.
@alexxa thanks for this!
Is it possible to extract this shell logic into a script? This way it is easier to maintain, can be run separately if needed, and we do not have to touch this CI file if we want to update it.
alexxa
force-pushed
the
installation
branch
2 times, most recently
from
a1f338d to
f106e0e
Compare
* Add missing 'list_files' tool from __all__ * Refactor local/remote execution logic - Introduce a Command registry layer that stores all commands needed by a particular tool. - Split the parsing of command output and formatting the tool responses into separate functions. - Eliminate the use of psutil for local commands * Add a note to AGENTS about using Pydantic models over dataclasses * Fix block storage tests
* fix(network): add missing f-string prefix in error messages The error messages in get_network_connections() and get_listening_ports() were missing the f-string prefix, causing the literal placeholder text to be returned instead of the actual variable values. Signed-off-by: Major Hayden <major@redhat.com> * feat(commands): add validation to substitute_command_args Convert KeyError to ValueError with clear message when placeholders are missing from kwargs. Also validate that no placeholders remain after substitution (catches edge cases like nested braces). This enables fail-fast behavior when command templates are misconfigured, making debugging easier. Signed-off-by: Major Hayden <major@redhat.com> --------- Signed-off-by: Major Hayden <major@redhat.com>
…ion (rhel-lightspeed#109) - Add mock_execute_with_fallback_for factory fixture to conftest.py - Create module-level mock_execute fixture in test_network.py - Combine local and remote success tests into a single parameterized test - Reduce code duplication while maintaining test coverage Signed-off-by: Major Hayden <fmajor@mhtx.net> Signed-off-by: Major Hayden <major@redhat.com>
…erization (rhel-lightspeed#111) Apply the same parameterization pattern from TestGetNetworkInterfaces to TestGetNetworkConnections and TestGetListeningPorts: - Reuse module-level mock_execute fixture instead of inline patches - Combine local/remote success tests into parameterized test - Combine command_fails/empty_output failure tests into parameterized test - Keep exception tests separate (different assertion pattern) Reduces test methods from 10 to 6 while maintaining full coverage. Signed-off-by: Major Hayden <major@redhat.com>
Reduce patch/app test coverage requirements and we don't need to test our tests. 😆
* Move architecture to separate document This only renders correctly on GitHub and looks out of place on PyPI. * Cleanup Installation doc - Remove use of uvx in favor of permanent instnallation with uv - Document all config options - Add a note about not needing to run the server directly in normal use * Add example SSH config for per-host connection options * Fix PyPI badge link
…ightspeed#113) Remove dead code that was set up for link-local IPv6 filtering tests that were never implemented: - Remove MockAddr, MockConnection, MockConnectionType classes - Remove make_mixed_connections_network/process/listening_ports functions - Remove LINK_LOCAL_FILTER_CASES_PROCESS/NETWORK constants - Remove unused socket import This brings tests/conftest.py to 100% coverage. Signed-off-by: Major Hayden <major@redhat.com>
…tently (rhel-lightspeed#119) * refactor(commands): unify command registry to use CommandGroup consistently - Wrap all commands in CommandGroup for consistency, eliminating mixed CommandSpec/CommandGroup types and isinstance() checks in consuming code - Add get_command_group() helper for tools that iterate over subcommands - Improve get_command() error messages to list available commands/subcommands - Update system_info.py to use get_command_group() instead of direct access - Add tests for get_command() and get_command_group() error cases Signed-off-by: Major Hayden <major@redhat.com> * test(storage): remove dead branches in test fixtures Simplify setup_test_directory and setup_test_files fixtures by removing conditional size checks. The `if size > 0` branches were never exercised since no tests passed size=0, causing codecov to flag them as dead code. Since write_text("x" * 0) produces an empty string and works correctly, the conditionals were unnecessary complexity. Signed-off-by: Major Hayden <major@redhat.com> --------- Signed-off-by: Major Hayden <major@redhat.com>
…d#120) * build(deps): add pytest-randomly to test dependencies Randomizes test execution order to help detect hidden test interdependencies that could cause flaky tests. Signed-off-by: Major Hayden <major@redhat.com> * fix(ssh): prevent mutation of caller's command list Copy command list before modifying command[0] with resolved binary path. This prevents permanent mutation of CommandSpec.args in the global COMMANDS registry when tools pass cmd.args directly to execute_command. Bug discovered by pytest-randomly exposing test order dependency. Signed-off-by: Major Hayden <major@redhat.com> --------- Signed-off-by: Major Hayden <major@redhat.com>
due to CFFI not supporting the free-threaded build of CPython
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
6 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.
No description provided.