test(e2e): bump CLI timeouts from 5s to 30s for slow CI runners - #6
Conversation
darwin_amd64 GitHub runners exceed the 5s ProcessTimeoutError on simple `supsrc <cmd> --help` invocations: 8 e2e tests fail there, all 4 other test matrixes (linux_amd64, linux_arm64, darwin_arm64, windows_amd64) pass. 30s gives slow CI environments room while still catching genuine hangs (a help command taking >30s would indicate a real regression). The 10s timeouts are left alone — they already tolerate slow runners.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request increases the timeout for several CLI integration tests from 5 to 30 seconds to prevent failures in slower environments. The review feedback suggests defining a module-level constant for the timeout to improve maintainability and ensure consistency across the test suite. Additionally, it is recommended to mark these tests with the @pytest.mark.slow decorator to align with the project's handling of long-running tests.
| result = run( | ||
| [sys.executable, "-m", "supsrc.cli.main", "watch", "--help"], | ||
| timeout=5, | ||
| timeout=30, |
There was a problem hiding this comment.
The hardcoded timeout of 30 is duplicated across 10 test cases in this PR, which is a maintainability concern. Additionally, this creates an inconsistency with other CLI tests in the same file (e.g., lines 35, 50, 68) that still use a 10 second timeout for more complex operations like config show. If the environment is slow enough to require 30s for a --help command, the 10s limit for configuration loading is likely to cause intermittent failures.\n\nConsider defining a module-level constant (e.g., DEFAULT_CLI_TIMEOUT = 30) and applying it consistently to all CLI integration tests.
| result = run( | ||
| [sys.executable, "-m", "supsrc.cli.main", "nonexistent-command"], | ||
| timeout=5, | ||
| timeout=30, |
There was a problem hiding this comment.
The tests test_invalid_command_handling, test_version_command, test_help_command, test_cli_python_path_handling, and test_config_validation_error_reporting have had their timeouts increased to 30s but are not marked with the @pytest.mark.slow decorator.\n\nFor consistency with other long-running tests in this suite (e.g., test_watch_command_dry_run), these should also be marked as slow to allow developers to exclude them during rapid local testing cycles.
darwin_amd64 GitHub runners exceed the 5s ProcessTimeoutError on simple `supsrc <cmd> --help` invocations: 8 e2e tests fail there, all 4 other test matrixes (linux_amd64, linux_arm64, darwin_arm64, windows_amd64) pass. 30s gives slow CI environments room while still catching genuine hangs (a help command taking >30s would indicate a real regression). The 10s timeouts are left alone — they already tolerate slow runners.
1 participant
Unblocks ci.yml on darwin_amd64. 8 e2e CLI tests timed out at 5s on macOS Intel GitHub runners (
Command timed out after 5s: ... -m supsrc.cli.main <cmd> --help). All other 4 platforms green. 30s is forgiving enough for slow runners while still catching real regressions; the 10s timeouts already tolerate.