feat: add XLSX export capability to CLI - #10
Merged
Merged
Conversation
Co-Authored-By: martyy-code <nesalia.inc@gmail.com>
- Add openpyxl as a dependency for XLSX support - Create xlsx_export.py with posts_to_xlsx, comments_to_xlsx, and subreddits_to_xlsx functions - Add --format xlsx option to all data-fetching commands: - reddit browse, frontpage, home, best, search, post, comments, comment - reddit subreddit, subreddits (popular, search, new, gold, default) - openpyxl is imported lazily so CLI still works without it - If openpyxl is not installed and user tries xlsx export, show clear error message Co-Authored-By: martyy-code <nesalia.inc@gmail.com>
📝 WalkthroughWalkthroughAdds XLSX export support: new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as Typer CLI
participant API as Reddit API
participant Export as xlsx_export
participant FS as Filesystem
User->>CLI: run command (e.g., --format xlsx --output file.xlsx)
CLI->>API: fetch posts/comments/subreddits
API-->>CLI: return JSON data
CLI->>Export: posts_to_xlsx/comments_to_xlsx/subreddits_to_xlsx(data)
Export-->>CLI: return XLSX bytes
CLI->>FS: write bytes to `file.xlsx` (binary)
CLI-->>User: print "Exported N posts ..." / exit 0
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
reddit_cli/commands/navigation.py (1)
57-65: Consider centralizing the XLSX output branch used across command modulesThis branch is now repeated in multiple files (
browse.py,search.py,post.py,subreddit.py,comments.py,navigation.py). A shared helper would reduce maintenance drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@reddit_cli/commands/navigation.py` around lines 57 - 65, The repeated XLSX export branch (checking format_type == "xlsx", validating output_file, calling posts_to_xlsx(posts), writing bytes, and echoing the result) should be extracted into a shared helper to avoid duplication; create a utility function (e.g., export_posts_to_xlsx(posts, output_file)) that validates output_file, calls posts_to_xlsx(posts), writes the file and returns or logs the success message, then replace the inlined branch in functions/modules that handle format_type/output_file (references: format_type, output_file, posts, posts_to_xlsx) to call this new helper instead of duplicating the logic.reddit_cli/xlsx_export.py (1)
63-75: Extract shared column autosize logic to a helperThe same autosize block is duplicated three times. Consolidating it reduces drift and future bug-fix overhead.
Refactor sketch
+def _autosize_columns(ws, max_width: int = 50) -> None: + for column in ws.columns: + column_letter = column[0].column_letter + max_length = 0 + for cell in column: + cell_len = len(str(cell.value)) if cell.value is not None else 0 + if cell_len > max_length: + max_length = cell_len + ws.column_dimensions[column_letter].width = min(max_length + 2, max_width) @@ - # Auto-adjust column widths - for column in ws.columns: - ... + _autosize_columns(ws)Also applies to: 125-137, 180-192
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@reddit_cli/xlsx_export.py` around lines 63 - 75, Extract the duplicated autosize loop into a helper function (e.g., auto_adjust_column_widths(ws, max_width=50)) that takes a Worksheet and optional max width, moves the logic that iterates ws.columns, computes max_length from cell.value, clamps adjusted_width = min(max_length + 2, max_width), and assigns ws.column_dimensions[column_letter].width; then replace each of the three duplicated blocks with a call to auto_adjust_column_widths(ws) (or pass a different max_width when needed). Ensure the helper lives near other XLSX utilities and import/visibility are correct so references to ws, column_letter, and ws.column_dimensions continue to work.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pyproject.toml`:
- Line 7: Add the types-openpyxl package to the development dependencies so IDEs
and type checkers get stubs for openpyxl; update the pyproject.toml
dev-dependencies section to include "types-openpyxl" (no runtime import changes
needed) so files that import openpyxl (referenced in the dependencies list
containing "openpyxl>=3.1.0") gain type hints and improved mypy/IDE support.
In `@reddit_cli/xlsx_export.py`:
- Around line 71-72: Replace the three bare "except:" blocks in the autosize
loops with a specific exception catcher to avoid hiding errors and satisfy Ruff
E722; change each "except: pass" to "except Exception as e:" (or a more specific
exception type if you can determine it from the surrounding code) and handle it
by continuing or logging (e.g., ignore non-critical errors but do not use a bare
except). Locate the occurrences in reddit_cli/xlsx_export.py where the autosize
logic uses "except: pass" and update those blocks to "except Exception as e:"
(or an appropriate concrete exception) to preserve behavior while not swallowing
all exceptions.
- Around line 108-123: The recursive flatten_and_add function can hit Python
recursion limits for deep threads; replace it with an iterative traversal that
uses an explicit stack (or deque) to walk comments_list and call ws.append for
each comment (preserving fields comment.id, comment.author, comment.body,
comment.score, comment.created_utc, comment.parent_id, comment.link_id,
comment.depth). Implement the stack loop so when you pop a comment you append it
to the sheet and then push its replies onto the stack in the correct order
(reverse replies if you want to preserve original DFS order), removing the
recursive call to flatten_and_add and invoking the new iterative routine on
comments. Ensure you reference the existing symbols flatten_and_add (to
replace), comments_list/comments, comment.replies and ws.append so the change is
localized.
---
Nitpick comments:
In `@reddit_cli/commands/navigation.py`:
- Around line 57-65: The repeated XLSX export branch (checking format_type ==
"xlsx", validating output_file, calling posts_to_xlsx(posts), writing bytes, and
echoing the result) should be extracted into a shared helper to avoid
duplication; create a utility function (e.g., export_posts_to_xlsx(posts,
output_file)) that validates output_file, calls posts_to_xlsx(posts), writes the
file and returns or logs the success message, then replace the inlined branch in
functions/modules that handle format_type/output_file (references: format_type,
output_file, posts, posts_to_xlsx) to call this new helper instead of
duplicating the logic.
In `@reddit_cli/xlsx_export.py`:
- Around line 63-75: Extract the duplicated autosize loop into a helper function
(e.g., auto_adjust_column_widths(ws, max_width=50)) that takes a Worksheet and
optional max width, moves the logic that iterates ws.columns, computes
max_length from cell.value, clamps adjusted_width = min(max_length + 2,
max_width), and assigns ws.column_dimensions[column_letter].width; then replace
each of the three duplicated blocks with a call to auto_adjust_column_widths(ws)
(or pass a different max_width when needed). Ensure the helper lives near other
XLSX utilities and import/visibility are correct so references to ws,
column_letter, and ws.column_dimensions continue to work.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 425621e9-1517-4e34-90a1-aafef4dc3c62
📒 Files selected for processing (8)
pyproject.tomlreddit_cli/commands/browse.pyreddit_cli/commands/comments.pyreddit_cli/commands/navigation.pyreddit_cli/commands/post.pyreddit_cli/commands/search.pyreddit_cli/commands/subreddit.pyreddit_cli/xlsx_export.py
| readme = "README.md" | ||
| requires-python = ">=3.14" | ||
| dependencies = ["typer", "httpx", "pydantic"] | ||
| dependencies = ["typer", "httpx", "pydantic", "openpyxl>=3.1.0"] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Checking openpyxl dependency and typing stubs presence..."
rg -n --no-heading 'openpyxl|types-openpyxl' pyproject.toml reddit_cli/xlsx_export.pyRepository: AliiiBenn/reddit-cli
Length of output: 1227
🏁 Script executed:
cat -n pyproject.tomlRepository: AliiiBenn/reddit-cli
Length of output: 894
🏁 Script executed:
fd -e "mypy.ini" -o ".mypy.ini" -o "setup.cfg" | head -20Repository: AliiiBenn/reddit-cli
Length of output: 201
🏁 Script executed:
rg -n "mypy|disallow_untyped_calls|disallow_untyped_defs|ignore_missing_imports" pyproject.tomlRepository: AliiiBenn/reddit-cli
Length of output: 46
🌐 Web query:
types-openpyxl package availability PyPI typing stubs
💡 Result:
The types-openpyxl package, providing typing stubs for openpyxl, is available on PyPI. The latest version as of 2026-03-22 is 3.1.5.20260322, which targets openpyxl==3.1.5. It is part of the official Typeshed project (github.com/python/typeshed) and supports type checkers like mypy and pyright. Install via pip install types-openpyxl. An older, third-party alternative called openpyxl-stubs (last updated 2023) also exists but is less recommended due to lack of recent maintenance.
Citations:
- 1: https://pypi.org/project/types-openpyxl/
- 2: https://pypistats.org/packages/types-openpyxl
- 3: https://pypi.org/project/openpyxl/
- 4: https://pypi.org/project/openpyxl-stubs/
- 5: https://www.wheelodex.org/projects/types-openpyxl/
🏁 Script executed:
fd -type f -name "*.yml" -o -name "*.yaml" | grep -E "(github|gitlab|ci)" | head -20Repository: AliiiBenn/reddit-cli
Length of output: 234
🏁 Script executed:
find . -maxdepth 3 -type f \( -name "*.yml" -o -name "*.yaml" -o -name ".mypy.ini" -o -name "setup.cfg" -o -name "pyproject.toml" \) | xargs rg -l "mypy" 2>/dev/nullRepository: AliiiBenn/reddit-cli
Length of output: 124
🏁 Script executed:
ls -la .github/workflows/ 2>/dev/null || echo "No .github/workflows directory found"Repository: AliiiBenn/reddit-cli
Length of output: 410
🏁 Script executed:
rg "disallow.*untyped|ignore.*missing.*imports" --type yaml --type tomlRepository: AliiiBenn/reddit-cli
Length of output: 46
🏁 Script executed:
cat -n .github/workflows/mypy.ymlRepository: AliiiBenn/reddit-cli
Length of output: 733
🏁 Script executed:
find . -maxdepth 2 -name "setup.cfg" -o -name ".mypy.ini"Repository: AliiiBenn/reddit-cli
Length of output: 46
🏁 Script executed:
rg "tool\.mypy\]|disallow_untyped" --multilineRepository: AliiiBenn/reddit-cli
Length of output: 46
🏁 Script executed:
rg "import_untyped|disallow.*untyped" .Repository: AliiiBenn/reddit-cli
Length of output: 46
Add types-openpyxl to dev dependencies for better type checking
The suggested patch is recommended to improve type coverage for the openpyxl dependency. While mypy is currently running without strict type-checking flags, adding types-openpyxl from Typeshed will enable better IDE support, autocomplete, and future-proof the project against stricter type-checking configurations.
Suggested patch
[project.optional-dependencies]
-dev = ["pytest", "pytest-asyncio", "respx", "httpx"]
+dev = ["pytest", "pytest-asyncio", "respx", "httpx", "types-openpyxl"]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pyproject.toml` at line 7, Add the types-openpyxl package to the development
dependencies so IDEs and type checkers get stubs for openpyxl; update the
pyproject.toml dev-dependencies section to include "types-openpyxl" (no runtime
import changes needed) so files that import openpyxl (referenced in the
dependencies list containing "openpyxl>=3.1.0") gain type hints and improved
mypy/IDE support.
Comment on lines
+71
to
+72
| except: | ||
| pass |
There was a problem hiding this comment.
Replace bare except in autosize loops (Ruff E722 blocker)
Line 71, Line 133, and Line 188 use bare except, which is currently failing CI and can hide real bugs.
Suggested patch
- for cell in column:
- try:
- if len(str(cell.value)) > max_length:
- max_length = len(str(cell.value))
- except:
- pass
+ for cell in column:
+ cell_len = len(str(cell.value)) if cell.value is not None else 0
+ if cell_len > max_length:
+ max_length = cell_lenAlso applies to: 133-134, 188-189
🧰 Tools
🪛 GitHub Actions: Ruff
[error] 71-72: ruff (E722) Do not use bare except.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@reddit_cli/xlsx_export.py` around lines 71 - 72, Replace the three bare
"except:" blocks in the autosize loops with a specific exception catcher to
avoid hiding errors and satisfy Ruff E722; change each "except: pass" to "except
Exception as e:" (or a more specific exception type if you can determine it from
the surrounding code) and handle it by continuing or logging (e.g., ignore
non-critical errors but do not use a bare except). Locate the occurrences in
reddit_cli/xlsx_export.py where the autosize logic uses "except: pass" and
update those blocks to "except Exception as e:" (or an appropriate concrete
exception) to preserve behavior while not swallowing all exceptions.
Comment on lines
+108
to
+123
| def flatten_and_add(comments_list: list[Comment]) -> None: | ||
| for comment in comments_list: | ||
| ws.append([ | ||
| comment.id, | ||
| comment.author, | ||
| comment.body, | ||
| comment.score, | ||
| comment.created_utc, | ||
| comment.parent_id, | ||
| comment.link_id, | ||
| comment.depth, | ||
| ]) | ||
| if comment.replies: | ||
| flatten_and_add(comment.replies) | ||
|
|
||
| flatten_and_add(comments) |
There was a problem hiding this comment.
Recursive comment flattening can crash on deep threads
The recursive flatten_and_add can hit Python recursion limits on deeply nested trees (runtime failure risk).
Suggested iterative traversal
- def flatten_and_add(comments_list: list[Comment]) -> None:
- for comment in comments_list:
- ws.append([
- comment.id,
- comment.author,
- comment.body,
- comment.score,
- comment.created_utc,
- comment.parent_id,
- comment.link_id,
- comment.depth,
- ])
- if comment.replies:
- flatten_and_add(comment.replies)
+ def flatten_and_add(comments_list: list[Comment]) -> None:
+ stack = list(reversed(comments_list))
+ while stack:
+ comment = stack.pop()
+ ws.append([
+ comment.id,
+ comment.author,
+ comment.body,
+ comment.score,
+ comment.created_utc,
+ comment.parent_id,
+ comment.link_id,
+ comment.depth,
+ ])
+ if comment.replies:
+ stack.extend(reversed(comment.replies))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def flatten_and_add(comments_list: list[Comment]) -> None: | |
| for comment in comments_list: | |
| ws.append([ | |
| comment.id, | |
| comment.author, | |
| comment.body, | |
| comment.score, | |
| comment.created_utc, | |
| comment.parent_id, | |
| comment.link_id, | |
| comment.depth, | |
| ]) | |
| if comment.replies: | |
| flatten_and_add(comment.replies) | |
| flatten_and_add(comments) | |
| def flatten_and_add(comments_list: list[Comment]) -> None: | |
| stack = list(reversed(comments_list)) | |
| while stack: | |
| comment = stack.pop() | |
| ws.append([ | |
| comment.id, | |
| comment.author, | |
| comment.body, | |
| comment.score, | |
| comment.created_utc, | |
| comment.parent_id, | |
| comment.link_id, | |
| comment.depth, | |
| ]) | |
| if comment.replies: | |
| stack.extend(reversed(comment.replies)) | |
| flatten_and_add(comments) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@reddit_cli/xlsx_export.py` around lines 108 - 123, The recursive
flatten_and_add function can hit Python recursion limits for deep threads;
replace it with an iterative traversal that uses an explicit stack (or deque) to
walk comments_list and call ws.append for each comment (preserving fields
comment.id, comment.author, comment.body, comment.score, comment.created_utc,
comment.parent_id, comment.link_id, comment.depth). Implement the stack loop so
when you pop a comment you append it to the sheet and then push its replies onto
the stack in the correct order (reverse replies if you want to preserve original
DFS order), removing the recursive call to flatten_and_add and invoking the new
iterative routine on comments. Ensure you reference the existing symbols
flatten_and_add (to replace), comments_list/comments, comment.replies and
ws.append so the change is localized.
Added 31 new tests to increase coverage for search.py from 57% to 99%: - Format options: sql, csv, xlsx (with/without output file) - Output parameter handling for all formats - Limit validation (zero, negative, above 100) - Sort validation (invalid value) - Period validation (invalid value) - Cursor display (after, before, both) - Error handling (404, 429, 500, 403) - Combined parameter tests Co-Authored-By: martyy-code <nesalia.inc@gmail.com>
Co-Authored-By: martyy-code <nesalia.inc@gmail.com>
Add 41 new tests for navigation commands (frontpage, home, best) to increase coverage from 51% to 97%. Coverage areas added: - All format options (display, sql, csv, xlsx) - Output file handling - All sort values (hot, new, top, rising, controversial, gilded) - All period values (day, week, month, year, all) - Empty results handling - Validation errors (invalid sort, period, limit) - Error handling (404, 500, 403, 429) - Display format with cursors (after, before, both) Co-Authored-By: martyy-code <nesalia.inc@gmail.com>
Co-Authored-By: martyy-code <nesalia.inc@gmail.com>
Add comprehensive tests for reddit_cli/errors.py covering: - handle_api_error with httpx.TimeoutException, httpx.ConnectError - handle_api_error with httpx.HTTPStatusError (400, 401, 403, 404, 429, 500, 502, 503, other) - handle_api_error with generic Exception and typer.Exit - handle_validation_error with various valid values lists - handle_interrupt exit code - Exit code constant verification Coverage increased from 60% to 100% Co-Authored-By: martyy-code <nesalia.inc@gmail.com>
Co-Authored-By: martyy-code <nesalia.inc@gmail.com>
Co-Authored-By: martyy-code <nesalia.inc@gmail.com>
Add comprehensive tests for reddit_cli/export.py covering: - escape_sql_value with various inputs - post_to_sql_insert and post_to_csv_row - comment_to_sql_insert and comment_to_csv_row - subreddit_to_sql_insert and subreddit_to_csv_row - CSV header functions - Edge cases (empty strings, zero values) Coverage increased from 52% to 100% Co-Authored-By: martyy-code <nesalia.inc@gmail.com>
Add comprehensive unit tests for PostsClient covering: - list_posts method with various parameters (sort, limit, period, cursors) - get_post method with t3_ prefix handling - search_posts method with subreddit restriction - get_sticky and get_random methods - get_duplicates method with list/ fallback formats - Edge cases for missing data keys and malformed responses Coverage increased from 65% to 97% Co-Authored-By: martyy-code <nesalia.inc@gmail.com>
Add comprehensive tests to reach 80%+ coverage target: - test_errors.py: 20 tests for error handling - test_export.py: 22 tests for SQL/CSV export - test_xlsx_export.py: 20 tests for XLSX export - test_posts.py: 29 tests for posts client - test_cli_browse.py: 35 additional tests - test_cli_comments.py: 20 additional tests - test_cli_navigation.py: 41 additional tests - test_cli_search.py: 31 additional tests - test_cli_subreddit.py: 37 additional tests Total: 370 tests, 95% coverage (up from 61%, 133 tests) Co-Authored-By: martyy-code <nesalia.inc@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (10)
tests/test_posts.py (1)
294-300: This is testing extra-key tolerance, not malformed post data.Adding
extra_fieldstill leaves the payload valid forPost(**...), so the test name is misleading. Either rename it to reflect ignored extras or make the fixture genuinely malformed and assert the expected behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_posts.py` around lines 294 - 300, The test name claims to verify malformed post data but only adds an extra key that the Post model tolerates; either rename the test (e.g., test_posts_client_ignores_extra_fields) to reflect that it ensures extra keys are ignored, or change the fixture to be truly malformed (for example remove or null out a required field like the post "id" or set an invalid type) and then assert PostsClient.list_posts (or the Post model construction) raises or handles the error as expected; update the test function name or assertions accordingly and keep references to the MockRedditClient, PostsClient, and the sample_posts_response fixture so the intent is clear.tests/test_cli_subreddit.py (1)
73-76: Make the validation-path assertions exact.These cases only check
result.exit_code != 0, so they would still pass if validation regressed back to a generic exit code1. Since invalid sort/format values are supposed to flow throughhandle_validation_error, assertresult.exit_code == 2and keep a message check.Also applies to: 147-149, 159-163, 223-227, 276-280, 329-333, 382-386
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_cli_subreddit.py` around lines 73 - 76, The tests (e.g., test_subreddit_invalid_format) currently assert only result.exit_code != 0; update each invalid-input test to assert the exact validation exit code and message by asserting result.exit_code == 2 and checking the output contains the validation error text emitted by handle_validation_error (for example verify result.output includes the expected "Invalid value for '--format'" or similar message). Locate tests calling runner.invoke(app, ["subreddit", ...]) with invalid sort/format values (e.g., test_subreddit_invalid_format and the similar cases around the other ranges) and replace the loose exit-code check with result.exit_code == 2 and an explicit substring assertion on result.output to confirm the validation path was used.tests/test_cli_comments.py (1)
174-182: Remove the stale exit-code notes.The notes on Line 180 and Line 266 now contradict both the assertions and
reddit_cli/errors.py, which re-raisestyper.Exitunchanged. Leaving them in place will mislead the next person touching these tests.Also applies to: 256-268
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_cli_comments.py` around lines 174 - 182, Remove the stale explanatory note that says "Note: exit code is 1 (not 2) due to typer.Exit being caught by except Exception" from the test_comments_invalid_format test (function test_comments_invalid_format) and the similar stale note in the other test block around the comments tests (the block referenced in the review, lines shown in the diff 256-268); these comments contradict the actual assertions and reddit_cli/errors.py behavior, so simply delete those note lines so the tests and inline comments are consistent with the re-raised typer.Exit and the assertions asserting exit_code == 2.tests/test_xlsx_export.py (7)
346-363: Consider simplifying the import mock approach.The current approach of patching
builtins.__import__is functional but fragile—it can inadvertently break other imports during the test. A simpler approach usespatch.dictonsys.modules:♻️ Simpler alternative using patch.dict
def test_check_openpyxl_not_available(self): """_check_openpyxl should raise ImportError when openpyxl is not available.""" - import sys - # Temporarily remove openpyxl from sys.modules - saved_openpyxl = sys.modules.pop("openpyxl", None) - # Patch the import side_effect to raise ImportError - def mock_import(name, globals=None, locals=None, fromlist=(), level=0): - if name == "openpyxl" or (isinstance(name, str) and name.startswith("openpyxl.")): - raise ImportError(f"No module named '{name}'") - return __import__(name, globals, locals, fromlist, level) - try: - with patch("builtins.__import__", side_effect=mock_import): - with pytest.raises(ImportError, match="openpyxl is required"): - xlsx_export._check_openpyxl() - finally: - # Restore openpyxl if it was there - if saved_openpyxl is not None: - sys.modules["openpyxl"] = saved_openpyxl + import sys + with patch.dict(sys.modules, {"openpyxl": None}): + with pytest.raises(ImportError, match="openpyxl is required"): + xlsx_export._check_openpyxl()Note: Setting a module to
Noneinsys.modulescausesimportto raiseImportError.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_xlsx_export.py` around lines 346 - 363, Replace the fragile custom __import__ patch in test_check_openpyxl_not_available with a patch.dict approach: remove/mask "openpyxl" in sys.modules using patch.dict to set sys.modules["openpyxl"]=None (or pop it) so import raises ImportError, then call xlsx_export._check_openpyxl() inside pytest.raises(ImportError, match="openpyxl is required"); ensure you restore any saved_openpyxl after the test. This changes the test_test_check_openpyxl_not_available test to use patch.dict on sys.modules instead of patching builtins.__import__.
10-31: Tests verify output type but not content correctness.All tests only assert that the result is
byteswith length > 0. Consider adding at least one test per function that loads the XLSX and verifies headers and cell values:💡 Example: Verify XLSX content
def test_posts_to_xlsx_content_verification(self): """posts_to_xlsx should produce XLSX with correct headers and data.""" import io from openpyxl import load_workbook posts = [ Post( id="abc123", title="Test Post Title", author="testuser", subreddit="python", score=100, num_comments=42, permalink="/r/python/comments/abc123/test/", url="https://example.com", created_utc=1704067200.0, selftext="Test content", ) ] result = xlsx_export.posts_to_xlsx(posts) wb = load_workbook(io.BytesIO(result)) ws = wb.active # Verify headers exist in first row headers = [cell.value for cell in ws[1]] assert "title" in headers or "Title" in headers # Verify data row exists assert ws.max_row >= 2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_xlsx_export.py` around lines 10 - 31, Add a content verification test to TestPostsToXlsx that goes beyond type/length checks: call xlsx_export.posts_to_xlsx with a sample Post, open the returned bytes with openpyxl (load_workbook + io.BytesIO), assert the expected header row values (e.g., "title" / "Title" and other column names produced by posts_to_xlsx) and assert the first data row contains the sample post's values (e.g., title, author, score) and that ws.max_row >= 2; name the test method test_posts_to_xlsx_content_verification and keep the existing sample Post fixture to locate usage in the file.
71-88: Custom sheet name test doesn't verify the sheet name was applied.The test passes
sheet_name="CustomSheet"but only checks that bytes are returned. Consider verifying the actual sheet name:💡 Suggested verification
def test_posts_to_xlsx_custom_sheet_name(self): """posts_to_xlsx should use custom sheet name.""" import io from openpyxl import load_workbook posts = [ Post( id="abc123", title="Test Post", # ... other fields ) ] result = xlsx_export.posts_to_xlsx(posts, sheet_name="CustomSheet") wb = load_workbook(io.BytesIO(result)) assert wb.active.title == "CustomSheet"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_xlsx_export.py` around lines 71 - 88, Update the test_posts_to_xlsx_custom_sheet_name test to verify the sheet name produced by xlsx_export.posts_to_xlsx rather than only checking for bytes; call xlsx_export.posts_to_xlsx(posts, sheet_name="CustomSheet"), load the resulting bytes into an openpyxl workbook (e.g., via io.BytesIO and openpyxl.load_workbook) and assert the workbook's active sheet title equals "CustomSheet" to ensure sheet_name is applied.
225-241: Test data doesn't contain actual special characters.Similar to the posts test, this claims to test "quotes and newlines and tabs" but the body contains plain text. Include actual special characters:
💡 Suggested improvement
comments = [ Comment( id="special123", author="user_special", - body="Comment with quotes and newlines and tabs", + body='Comment with "quotes"\nand newlines\tand tabs', score=1, created_utc=1704067200.0, parent_id="t3_post1", link_id="t3_post1", depth=0, ) ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_xlsx_export.py` around lines 225 - 241, The test test_comments_to_xlsx_special_characters currently describes special characters but uses plain text; update the Comment instance in the comments list (created in this test) so its body contains real special characters (e.g., double/single quotes, newline \n, tab \t and any Unicode like “—” or emoji) before calling xlsx_export.comments_to_xlsx; ensure the variable name comments and the call to xlsx_export.comments_to_xlsx remain unchanged so the test validates actual handling of special characters.
89-107: Test data doesn't actually contain special characters.The test claims to verify handling of "quotes and newlines" but the
titleandselftextfields contain plain text without actual special characters (no",',\n,\t). Consider including actual special characters to validate proper escaping/handling:💡 Suggested improvement
posts = [ Post( id="special123", - title="Post with quotes and newlines", + title='Post with "quotes" and\nnewlines', author="user_special", subreddit="test", score=1, num_comments=0, permalink="/r/test/comments/special123/test/", url="https://example.com", created_utc=1704067200.0, - selftext="Selftext with single quotes and unicode", + selftext="Selftext with 'single quotes'\tand\ttabs", ) ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_xlsx_export.py` around lines 89 - 107, The test_posts_to_xlsx_special_characters test uses plain text but should include real special characters to validate escaping; update the Post instance in test_posts_to_xlsx_special_characters so its title and selftext contain actual quotes, newlines, tabs, and Unicode (e.g. double quotes ", single quotes ', newline \n, tab \t, and an emoji or non-ASCII character), then run the same assertions against xlsx_export.posts_to_xlsx to ensure the XLSX bytes output handles these characters correctly.
109-127: Test data lacks actual unicode/emoji characters.The test is titled "unicode characters" but
titleandselftextcontain only ASCII text. Consider adding actual unicode:💡 Suggested improvement
posts = [ Post( id="unicode123", - title="Post with emoji and unicode", + title="Post with emoji 🐍 and unicode", author="user_unicode", subreddit="test", score=50, num_comments=5, permalink="/r/test/comments/unicode123/test/", url="https://example.com", created_utc=1704067200.0, - selftext="Content with accent", + selftext="Content with accénts and émojis 🎉", ) ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_xlsx_export.py` around lines 109 - 127, The test test_posts_to_xlsx_unicode_characters currently doesn't include real Unicode; update the Post test data used by xlsx_export.posts_to_xlsx to include actual non-ASCII characters (e.g., emoji like "😀" or "🔥", accented letters like "é", or other Unicode symbols) in the title and selftext fields so the test truly verifies Unicode handling; locate the Post construction in test_posts_to_xlsx_unicode_characters and replace or augment the title/selftext values with those Unicode characters.
307-335: Special character and unicode tests lack actual special/unicode characters.Both
test_subreddits_to_xlsx_special_charactersandtest_subreddits_to_xlsx_unicode_charactersuse plain ASCII text in their test data. Consider:💡 Suggested improvement
def test_subreddits_to_xlsx_special_characters(self): """subreddits_to_xlsx should handle special characters in description.""" subreddits = [ Subreddit( id="special123", display_name="test", title="Test Subreddit", - description="Description with quotes and newlines", + description='Description with "quotes"\nand newlines', subscribers=1000, active_users=100, ) ] # ... def test_subreddits_to_xlsx_unicode_characters(self): """subreddits_to_xlsx should handle unicode characters.""" subreddits = [ Subreddit( id="unicode123", display_name="test", - title="Test Subreddit with unicode", - description="Description with accent", + title="Test Subreddit with unicode 日本語", + description="Description with accénts and émoji 🎮", subscribers=1000, active_users=100, ) ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_xlsx_export.py` around lines 307 - 335, The two tests (test_subreddits_to_xlsx_special_characters and test_subreddits_to_xlsx_unicode_characters) are using only plain ASCII; update the Subreddit instances passed into xlsx_export.subreddits_to_xlsx to include real special characters (e.g., embedded quotes, newlines, tabs) in the description/title for the "special_characters" test and real Unicode (accented letters, non-Latin scripts or emoji) in the description/title for the "unicode_characters" test so the tests actually validate handling of those cases while keeping the same assertion that the result is bytes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/test_cli_browse.py`:
- Line 2: Remove the unused import statement "import os" from the top of the
tests/test_cli_browse.py module because it triggers Ruff F401; locate the import
in the module (the standalone "import os" line) and delete it, then re-run the
linters/tests to confirm the unused-import error is resolved.
In `@tests/test_cli_navigation.py`:
- Around line 127-151: Remove the duplicate test class definition: keep a single
TestNavigationDisplayFormat and consolidate or remove the duplicated block that
re-declares test_frontpage_display_shows_after_cursor; locate the two identical
class definitions named TestNavigationDisplayFormat (and the method
test_frontpage_display_shows_after_cursor) and delete the redundant second
definition so pytest/Ruff F811 no longer reports a redefinition.
In `@tests/test_cli_post.py`:
- Around line 110-111: The second class named TestPost redefines the first and
hides the initial smoke tests; rename the second TestPost (or merge its test
methods into the original TestPost) so test discovery doesn't lose the baseline
assertions. Locate the duplicate class definition (symbol: TestPost) in
tests/test_cli_post.py and either change its class name to something unique
(e.g., TestPostIntegration) and update any references, or move its methods into
the original TestPost class to consolidate tests while preserving all
assertions.
In `@tests/test_cli_subreddit.py`:
- Line 6: Remove the unused import "CliRunner" by deleting the line `from
typer.testing import CliRunner` in the test module; ensure no other references
to CliRunner remain (search for "CliRunner" in the file) and run the
linter/tests to verify Ruff F401 is resolved.
In `@tests/test_export.py`:
- Line 2: Remove the unused import "pytest" from tests/test_export.py: locate
the top-level import statement "import pytest" and delete it so the module no
longer contains an unused symbol that triggers Ruff F401.
- Around line 173-207: Two pairs of tests are duplicated causing redefinition
errors: remove the repeated definitions of test_subreddit_csv_header_format and
test_subreddit_csv_header_has_correct_field_count so each appears only once, and
consolidate the two test_subreddit_with_zero_subscribers functions into a single
test; when consolidating use the correct Subreddit field name (active_users)
consistent with the rest of the code, call subreddit_csv_header(),
subreddit_to_sql_insert(subreddit) and subreddit_to_csv_row(subreddit) as
before, and ensure the assertions check for both ", 0, 0" and ",0,0" variants as
originally intended; keep the canonical header string
"id,display_name,title,description,subscribers,active_users" and a single pair
of header format and field-count tests.
In `@tests/test_posts.py`:
- Around line 6-10: The MockRedditClient currently ignores its inputs so
request-shape tests cannot fail; modify class MockRedditClient to record each
call by adding a calls list (e.g., self.calls = []) in __init__ and append a
tuple of (path, params) inside async get(path, params=None) before returning
self._response_data, then update the option-specific tests to assert the last
call matches the expected request (for example assert mock_client.calls[-1] ==
("/r/python/new.json", {"limit": 25})) so sort/limit/period/subreddit and t3_
prefix cases validate the built URL/query.
---
Nitpick comments:
In `@tests/test_cli_comments.py`:
- Around line 174-182: Remove the stale explanatory note that says "Note: exit
code is 1 (not 2) due to typer.Exit being caught by except Exception" from the
test_comments_invalid_format test (function test_comments_invalid_format) and
the similar stale note in the other test block around the comments tests (the
block referenced in the review, lines shown in the diff 256-268); these comments
contradict the actual assertions and reddit_cli/errors.py behavior, so simply
delete those note lines so the tests and inline comments are consistent with the
re-raised typer.Exit and the assertions asserting exit_code == 2.
In `@tests/test_cli_subreddit.py`:
- Around line 73-76: The tests (e.g., test_subreddit_invalid_format) currently
assert only result.exit_code != 0; update each invalid-input test to assert the
exact validation exit code and message by asserting result.exit_code == 2 and
checking the output contains the validation error text emitted by
handle_validation_error (for example verify result.output includes the expected
"Invalid value for '--format'" or similar message). Locate tests calling
runner.invoke(app, ["subreddit", ...]) with invalid sort/format values (e.g.,
test_subreddit_invalid_format and the similar cases around the other ranges) and
replace the loose exit-code check with result.exit_code == 2 and an explicit
substring assertion on result.output to confirm the validation path was used.
In `@tests/test_posts.py`:
- Around line 294-300: The test name claims to verify malformed post data but
only adds an extra key that the Post model tolerates; either rename the test
(e.g., test_posts_client_ignores_extra_fields) to reflect that it ensures extra
keys are ignored, or change the fixture to be truly malformed (for example
remove or null out a required field like the post "id" or set an invalid type)
and then assert PostsClient.list_posts (or the Post model construction) raises
or handles the error as expected; update the test function name or assertions
accordingly and keep references to the MockRedditClient, PostsClient, and the
sample_posts_response fixture so the intent is clear.
In `@tests/test_xlsx_export.py`:
- Around line 346-363: Replace the fragile custom __import__ patch in
test_check_openpyxl_not_available with a patch.dict approach: remove/mask
"openpyxl" in sys.modules using patch.dict to set sys.modules["openpyxl"]=None
(or pop it) so import raises ImportError, then call
xlsx_export._check_openpyxl() inside pytest.raises(ImportError, match="openpyxl
is required"); ensure you restore any saved_openpyxl after the test. This
changes the test_test_check_openpyxl_not_available test to use patch.dict on
sys.modules instead of patching builtins.__import__.
- Around line 10-31: Add a content verification test to TestPostsToXlsx that
goes beyond type/length checks: call xlsx_export.posts_to_xlsx with a sample
Post, open the returned bytes with openpyxl (load_workbook + io.BytesIO), assert
the expected header row values (e.g., "title" / "Title" and other column names
produced by posts_to_xlsx) and assert the first data row contains the sample
post's values (e.g., title, author, score) and that ws.max_row >= 2; name the
test method test_posts_to_xlsx_content_verification and keep the existing sample
Post fixture to locate usage in the file.
- Around line 71-88: Update the test_posts_to_xlsx_custom_sheet_name test to
verify the sheet name produced by xlsx_export.posts_to_xlsx rather than only
checking for bytes; call xlsx_export.posts_to_xlsx(posts,
sheet_name="CustomSheet"), load the resulting bytes into an openpyxl workbook
(e.g., via io.BytesIO and openpyxl.load_workbook) and assert the workbook's
active sheet title equals "CustomSheet" to ensure sheet_name is applied.
- Around line 225-241: The test test_comments_to_xlsx_special_characters
currently describes special characters but uses plain text; update the Comment
instance in the comments list (created in this test) so its body contains real
special characters (e.g., double/single quotes, newline \n, tab \t and any
Unicode like “—” or emoji) before calling xlsx_export.comments_to_xlsx; ensure
the variable name comments and the call to xlsx_export.comments_to_xlsx remain
unchanged so the test validates actual handling of special characters.
- Around line 89-107: The test_posts_to_xlsx_special_characters test uses plain
text but should include real special characters to validate escaping; update the
Post instance in test_posts_to_xlsx_special_characters so its title and selftext
contain actual quotes, newlines, tabs, and Unicode (e.g. double quotes ", single
quotes ', newline \n, tab \t, and an emoji or non-ASCII character), then run the
same assertions against xlsx_export.posts_to_xlsx to ensure the XLSX bytes
output handles these characters correctly.
- Around line 109-127: The test test_posts_to_xlsx_unicode_characters currently
doesn't include real Unicode; update the Post test data used by
xlsx_export.posts_to_xlsx to include actual non-ASCII characters (e.g., emoji
like "😀" or "🔥", accented letters like "é", or other Unicode symbols) in the
title and selftext fields so the test truly verifies Unicode handling; locate
the Post construction in test_posts_to_xlsx_unicode_characters and replace or
augment the title/selftext values with those Unicode characters.
- Around line 307-335: The two tests (test_subreddits_to_xlsx_special_characters
and test_subreddits_to_xlsx_unicode_characters) are using only plain ASCII;
update the Subreddit instances passed into xlsx_export.subreddits_to_xlsx to
include real special characters (e.g., embedded quotes, newlines, tabs) in the
description/title for the "special_characters" test and real Unicode (accented
letters, non-Latin scripts or emoji) in the description/title for the
"unicode_characters" test so the tests actually validate handling of those cases
while keeping the same assertion that the result is bytes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0d233667-31c3-404a-89fe-f41770e7fd69
⛔ Files ignored due to path filters (11)
tests/__pycache__/test_cli_browse.cpython-314-pytest-9.0.2.pycis excluded by!**/*.pyctests/__pycache__/test_cli_comments.cpython-314-pytest-9.0.2.pycis excluded by!**/*.pyctests/__pycache__/test_cli_navigation.cpython-314-pytest-9.0.2.pycis excluded by!**/*.pyctests/__pycache__/test_cli_post.cpython-314-pytest-9.0.2.pycis excluded by!**/*.pyctests/__pycache__/test_cli_search.cpython-314-pytest-9.0.2.pycis excluded by!**/*.pyctests/__pycache__/test_cli_subreddit.cpython-314-pytest-9.0.2.pycis excluded by!**/*.pyctests/__pycache__/test_errors.cpython-314-pytest-9.0.2.pycis excluded by!**/*.pyctests/__pycache__/test_export.cpython-314-pytest-9.0.2.pycis excluded by!**/*.pyctests/__pycache__/test_posts.cpython-314-pytest-9.0.2.pycis excluded by!**/*.pyctests/__pycache__/test_xlsx_export.cpython-314-pytest-9.0.2.pycis excluded by!**/*.pyctests/__pycache__/test_xlsx_export.cpython-314.pycis excluded by!**/*.pyc
📒 Files selected for processing (11)
reddit_cli/errors.pytests/test_cli_browse.pytests/test_cli_comments.pytests/test_cli_navigation.pytests/test_cli_post.pytests/test_cli_search.pytests/test_cli_subreddit.pytests/test_errors.pytests/test_export.pytests/test_posts.pytests/test_xlsx_export.py
✅ Files skipped from review due to trivial changes (1)
- tests/test_errors.py
| @@ -1,4 +1,5 @@ | |||
| """Tests for browse commands: browse, --sticky, --random, --search.""" | |||
| import os | |||
There was a problem hiding this comment.
Remove the unused os import.
Ruff is already failing on Line 2 with F401, and this module never uses os.
🧰 Tools
🪛 GitHub Actions: Ruff
[error] 2-2: Ruff F401 os imported but unused
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_cli_browse.py` at line 2, Remove the unused import statement
"import os" from the top of the tests/test_cli_browse.py module because it
triggers Ruff F401; locate the import in the module (the standalone "import os"
line) and delete it, then re-run the linters/tests to confirm the unused-import
error is resolved.
Comment on lines
+127
to
+151
| class TestNavigationDisplayFormat: | ||
| """Test display format with cursors.""" | ||
|
|
||
| def test_frontpage_display_shows_after_cursor(self, runner, mock_reddit_base, sample_navigation_response): | ||
| sample_navigation_response["data"]["after"] = "t3_after123" | ||
| sample_navigation_response["data"]["before"] = None | ||
| mock_reddit_base.get("/r/reddit/hot.json").mock( | ||
| return_value=httpx.Response(200, json=sample_navigation_response) | ||
| ) | ||
| result = runner.invoke(app, ["frontpage"]) | ||
| assert result.exit_code == 0 | ||
| assert "After: t3_after123" in result.output | ||
|
|
||
| class TestNavigationDisplayFormat: | ||
| """Test display format with cursors.""" | ||
|
|
||
| def test_frontpage_display_shows_after_cursor(self, runner, mock_reddit_base, sample_navigation_response): | ||
| sample_navigation_response["data"]["after"] = "t3_after123" | ||
| sample_navigation_response["data"]["before"] = None | ||
| mock_reddit_base.get("/r/reddit/hot.json").mock( | ||
| return_value=httpx.Response(200, json=sample_navigation_response) | ||
| ) | ||
| result = runner.invoke(app, ["frontpage"]) | ||
| assert result.exit_code == 0 | ||
| assert "After: t3_after123" in result.output |
There was a problem hiding this comment.
Keep a single TestNavigationDisplayFormat class.
Line 140 redefines TestNavigationDisplayFormat, so the class introduced on Line 127 is discarded during collection. Ruff F811 is already catching it.
🧰 Tools
🪛 GitHub Actions: Ruff
[error] 140-141: Ruff F811 Redefinition of unused TestNavigationDisplayFormat (redefined from line 127)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_cli_navigation.py` around lines 127 - 151, Remove the duplicate
test class definition: keep a single TestNavigationDisplayFormat and consolidate
or remove the duplicated block that re-declares
test_frontpage_display_shows_after_cursor; locate the two identical class
definitions named TestNavigationDisplayFormat (and the method
test_frontpage_display_shows_after_cursor) and delete the redundant second
definition so pytest/Ruff F811 no longer reports a redefinition.
Comment on lines
+110
to
+111
| class TestPost: | ||
| """Test suite for post command.""" |
There was a problem hiding this comment.
Rename or merge the second TestPost class.
Line 110 rebinds TestPost, so the earlier smoke tests on Lines 40-79 stop being collected. Ruff F811 catches the redefinition, but the bigger problem is silently losing those baseline assertions.
♻️ Minimal fix
-class TestPost:
+class TestPostFormats:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| class TestPost: | |
| """Test suite for post command.""" | |
| class TestPostFormats: | |
| """Test suite for post command.""" |
🧰 Tools
🪛 GitHub Actions: Ruff
[error] 110-111: Ruff F811 Redefinition of unused TestPost (redefined from line 37)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_cli_post.py` around lines 110 - 111, The second class named
TestPost redefines the first and hides the initial smoke tests; rename the
second TestPost (or merge its test methods into the original TestPost) so test
discovery doesn't lose the baseline assertions. Locate the duplicate class
definition (symbol: TestPost) in tests/test_cli_post.py and either change its
class name to something unique (e.g., TestPostIntegration) and update any
references, or move its methods into the original TestPost class to consolidate
tests while preserving all assertions.
| import tempfile | ||
| import pytest | ||
| import httpx | ||
| from typer.testing import CliRunner |
There was a problem hiding this comment.
Remove the unused CliRunner import.
Ruff F401 on Line 6 matches what this file shows now: none of the tests use CliRunner directly anymore.
🧰 Tools
🪛 GitHub Actions: Ruff
[error] 6-6: Ruff F401 typer.testing.CliRunner imported but unused
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_cli_subreddit.py` at line 6, Remove the unused import "CliRunner"
by deleting the line `from typer.testing import CliRunner` in the test module;
ensure no other references to CliRunner remain (search for "CliRunner" in the
file) and run the linter/tests to verify Ruff F401 is resolved.
| @@ -0,0 +1,207 @@ | |||
| """Unit tests for export utilities (SQL and CSV formatting).""" | |||
| import pytest | |||
There was a problem hiding this comment.
Drop the unused pytest import.
Ruff is already failing on Line 2 with F401, and nothing in this module references pytest.
🧰 Tools
🪛 GitHub Actions: Ruff
[error] 2-2: Ruff F401 pytest imported but unused
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_export.py` at line 2, Remove the unused import "pytest" from
tests/test_export.py: locate the top-level import statement "import pytest" and
delete it so the module no longer contains an unused symbol that triggers Ruff
F401.
Comment on lines
+173
to
+207
| def test_subreddit_csv_header_format(self): | ||
| header = subreddit_csv_header() | ||
| assert header == "id,display_name,title,description,subscribers,active_users" | ||
|
|
||
| def test_subreddit_csv_header_has_correct_field_count(self): | ||
| fields = subreddit_csv_header().split(",") | ||
| assert len(fields) == 6 | ||
|
|
||
| def test_subreddit_csv_header_format(self): | ||
| header = subreddit_csv_header() | ||
| assert header == "id,display_name,title,description,subscribers,active_users" | ||
|
|
||
| def test_subreddit_csv_header_has_correct_field_count(self): | ||
| fields = subreddit_csv_header().split(",") | ||
| assert len(fields) == 6 | ||
|
|
||
|
|
||
| def test_subreddit_with_zero_subscribers(self): | ||
| subreddit = Subreddit(id="2qh13", display_name="test", | ||
| title="Test Subreddit", | ||
| description="Description", | ||
| subscribers=0, accounts_active=0) | ||
| sql = subreddit_to_sql_insert(subreddit) | ||
| csv = subreddit_to_csv_row(subreddit) | ||
| assert ", 0, 0" in sql or ",0,0" in sql | ||
| assert ",0,0" in csv | ||
| def test_subreddit_with_zero_subscribers(self): | ||
| subreddit = Subreddit(id="2qh13", display_name="test", | ||
| title="Test Subreddit", | ||
| description="Description", | ||
| subscribers=0, active_users=0) | ||
| sql = subreddit_to_sql_insert(subreddit) | ||
| csv = subreddit_to_csv_row(subreddit) | ||
| assert ", 0, 0" in sql or ",0,0" in sql | ||
| assert ",0,0" in csv |
There was a problem hiding this comment.
Deduplicate the repeated subreddit header/zero-subscriber tests.
Line 181, Line 185, and Line 199 redefine earlier methods in the same class, so the first versions are discarded at import time. That both trips Ruff F811 and removes some of the coverage this file is trying to add.
🧰 Tools
🪛 GitHub Actions: Ruff
[error] 181-182: Ruff F811 Redefinition of unused test_subreddit_csv_header_format (redefined from line 173)
[error] 185-187: Ruff F811 Redefinition of unused test_subreddit_csv_header_has_correct_field_count (redefined from line 177)
[error] 199-202: Ruff F811 Redefinition of unused test_subreddit_with_zero_subscribers (redefined from line 190)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_export.py` around lines 173 - 207, Two pairs of tests are
duplicated causing redefinition errors: remove the repeated definitions of
test_subreddit_csv_header_format and
test_subreddit_csv_header_has_correct_field_count so each appears only once, and
consolidate the two test_subreddit_with_zero_subscribers functions into a single
test; when consolidating use the correct Subreddit field name (active_users)
consistent with the rest of the code, call subreddit_csv_header(),
subreddit_to_sql_insert(subreddit) and subreddit_to_csv_row(subreddit) as
before, and ensure the assertions check for both ", 0, 0" and ",0,0" variants as
originally intended; keep the canonical header string
"id,display_name,title,description,subscribers,active_users" and a single pair
of header format and field-count tests.
Comment on lines
+6
to
+10
| class MockRedditClient: | ||
| def __init__(self, response_data=None): | ||
| self._response_data = response_data | ||
| async def get(self, path, params=None): | ||
| return self._response_data |
There was a problem hiding this comment.
Make the mock observable so the request-shape tests can actually fail.
MockRedditClient.get() ignores path and params, so the sort/limit/period/subreddit/t3_ prefix cases below still pass even if PostsClient builds the wrong URL or query string. Right now those tests only re-check parsing against a fixed payload. Record calls here and assert them in the option-specific cases.
Proposed change
class MockRedditClient:
def __init__(self, response_data=None):
self._response_data = response_data
+ self.calls: list[tuple[str, dict | None]] = []
+
async def get(self, path, params=None):
+ self.calls.append((path, params))
return self._response_dataThen add assertions like assert mock_client.calls[-1] == ("/r/python/new.json", {"limit": 25}) in the request-specific tests.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| class MockRedditClient: | |
| def __init__(self, response_data=None): | |
| self._response_data = response_data | |
| async def get(self, path, params=None): | |
| return self._response_data | |
| class MockRedditClient: | |
| def __init__(self, response_data=None): | |
| self._response_data = response_data | |
| self.calls: list[tuple[str, dict | None]] = [] | |
| async def get(self, path, params=None): | |
| self.calls.append((path, params)) | |
| return self._response_data |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_posts.py` around lines 6 - 10, The MockRedditClient currently
ignores its inputs so request-shape tests cannot fail; modify class
MockRedditClient to record each call by adding a calls list (e.g., self.calls =
[]) in __init__ and append a tuple of (path, params) inside async get(path,
params=None) before returning self._response_data, then update the
option-specific tests to assert the last call matches the expected request (for
example assert mock_client.calls[-1] == ("/r/python/new.json", {"limit": 25}))
so sort/limit/period/subreddit and t3_ prefix cases validate the built
URL/query.
3 tasks done
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
1 participant
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
openpyxlas a dependency for XLSX supportxlsx_export.pywith three functions:posts_to_xlsx(posts: list[Post], sheet_name: str) -> bytescomments_to_xlsx(comments: list[Comment], sheet_name: str) -> bytessubreddits_to_xlsx(subreddits: list[Subreddit], sheet_name: str) -> bytes--format xlsxoption to all data-fetching commandsCommands Updated
reddit browse- postsreddit frontpage/home/best- postsreddit search- postsreddit post- single postreddit comments- commentsreddit comment- single commentreddit subreddit- subreddit inforeddit subreddits(popular, search, new, gold, default) - subreddit listUsage Example
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
--format xlsxand--output; Excel files include headers and optimized column widths.--outputfor XLSX and reports an informative error when missing.