Validate report sheets instead of relying on tox exit code#175
Validate report sheets instead of relying on tox exit code#175woocheol-lge wants to merge 1 commit into
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds test infrastructure to validate Excel report output. The dependency ChangesExcel Output Validation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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.
🧹 Nitpick comments (2)
tests/test_fosslight_scanner.py (1)
205-205: 💤 Low valueConsider using Path.glob for consistency.
Since
output_diris already aPathobject, you could uselist(output_dir.glob("*.xlsx"))instead ofglob.glob(str(output_dir / "*.xlsx"))for consistency with the pathlib usage elsewhere in the test.♻️ Proposed refactor
- xlsx_files = glob.glob(str(output_dir / "*.xlsx")) + xlsx_files = list(output_dir.glob("*.xlsx")) + xlsx_files = [str(f) for f in xlsx_files] # Convert back to strings for _get_sheet_namesOr update
_get_sheet_namesto acceptPathobjects directly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_fosslight_scanner.py` at line 205, Replace the glob.glob call with a Path.glob-based approach by changing the xlsx file collection to use output_dir.glob("*.xlsx") (e.g., assign xlsx_files = list(output_dir.glob("*.xlsx"))) so the test consistently uses pathlib; alternatively, update the helper _get_sheet_names to accept Path objects so callers can pass Path instances directly (refer to xlsx_files, output_dir and _get_sheet_names to locate the change).requirements-dev.txt (1)
7-7: 💤 Low valueConsider pinning the openpyxl version for reproducible builds.
While the existing pattern in this file leaves development dependencies unpinned, specifying a minimum or exact version (e.g.,
openpyxl>=3.0.0) would prevent unexpected breakage if the library's API changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@requirements-dev.txt` at line 7, The dev dependency openpyxl is unpinned which can cause unreproducible builds; update the requirements-dev.txt entry for openpyxl to a pinned or minimum-version specifier (for example use openpyxl>=3.0.0 or an exact version like openpyxl==3.1.2) so installs are reproducible and less prone to breaking changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@requirements-dev.txt`:
- Line 7: The dev dependency openpyxl is unpinned which can cause unreproducible
builds; update the requirements-dev.txt entry for openpyxl to a pinned or
minimum-version specifier (for example use openpyxl>=3.0.0 or an exact version
like openpyxl==3.1.2) so installs are reproducible and less prone to breaking
changes.
In `@tests/test_fosslight_scanner.py`:
- Line 205: Replace the glob.glob call with a Path.glob-based approach by
changing the xlsx file collection to use output_dir.glob("*.xlsx") (e.g., assign
xlsx_files = list(output_dir.glob("*.xlsx"))) so the test consistently uses
pathlib; alternatively, update the helper _get_sheet_names to accept Path
objects so callers can pass Path instances directly (refer to xlsx_files,
output_dir and _get_sheet_names to locate the change).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3a6fa8b8-74ea-4c96-ac43-156eff207a56
📒 Files selected for processing (2)
requirements-dev.txttests/test_fosslight_scanner.py
woocheol-lge
force-pushed
the
e_test
branch
2 times, most recently
from
6871c0e to
97889e3
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_fosslight_scanner.py`:
- Around line 204-212: The test currently picks the first Excel file from
xlsx_files and may validate the wrong artifact; update the selection logic so
the test binds sheet validation to the intended report (e.g., filter xlsx_files
by the expected filename or unique pattern generated by the run, or choose the
most recently modified file) before calling _get_sheet_names; locate the
xlsx_files variable and the call to _get_sheet_names(...) and replace the naive
xlsx_files[0] usage with filtered/selected_report logic (by name pattern or
max(stat().st_mtime)) to guarantee the correct workbook is validated.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2c812c5b-4c4d-4d58-89eb-62ce9b4c55e7
📒 Files selected for processing (2)
requirements-dev.txttests/test_fosslight_scanner.py
Signed-off-by: woocheol <jayden6659@gmail.com>
1 participant
Description
Validate report sheets instead of relying on tox exit code
Summary by CodeRabbit
Chores
Tests