feat(integration-tests): Add compression action and verification infra.#2232
Open
quinntaylormitchell wants to merge 4 commits intoy-scope:mainfrom
Open
feat(integration-tests): Add compression action and verification infra.#2232quinntaylormitchell wants to merge 4 commits intoy-scope:mainfrom
quinntaylormitchell wants to merge 4 commits intoy-scope:mainfrom
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds typed command-arg models and verification primitives, plus new CLP compression and decompression test utilities that build command lines, run external scripts via ExternalAction, and verify outputs (including directory-tree comparison for unstructured datasets). Changes
Sequence Diagram(s)sequenceDiagram
participant TestRunner as Test Runner
participant ExternalAction as ExternalAction
participant Script as Compress/Decompress Script
participant FileSystem as Filesystem
TestRunner->>ExternalAction: construct CmdArgs (CompressArgs/DecompressArgs)
TestRunner->>ExternalAction: start action (cmd from args)
ExternalAction->>Script: execute command
Script->>FileSystem: read/write package and logs
Script-->>ExternalAction: exit code, stdout/stderr
ExternalAction-->>TestRunner: result (+ args)
TestRunner->>TestRunner: verify_compress_action: if unstructured -> run decompress action
TestRunner->>ExternalAction: start decompress action
ExternalAction->>Script: execute decompress
Script->>FileSystem: extract files to decompression dir
TestRunner->>FileSystem: compare original logs dir vs decompressed dir
TestRunner-->>TestRunner: return VerificationResult.ok()/fail()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Ruff (0.15.12)integration-tests/tests/package_tests/utils/compress.py�[1;31mruff failed�[0m 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. Review rate limit: 6/8 reviews remaining, refill in 12 minutes and 3 seconds.Comment |
quinntaylormitchell
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@integration-tests/tests/package_tests/utils/compress.py`:
- Around line 129-135: Remove the redundant check for
compress_action.completed_proc.returncode != 0 from the helper functions
_verify_compress_action_structured_logs and
_verify_compress_action_unstructured_logs (they currently return
VerificationResult.fail with format_action_failure_msg); verify_compress_action
already validates the return code before calling these helpers, so delete those
conditional blocks and their early-return failure logic, leaving the remaining
validation logic in those helper functions intact.
- Around line 161-165: Replace the pytest.fail() call used when
decompress_action.completed_proc.returncode != 0 with a return of
VerificationResult.fail(...) so the function consistently returns a
VerificationResult; use the same failure message (including
decompress_action.log_file_path) when constructing VerificationResult.fail(),
and update the function's docstring only if you intend to keep pytest.fail()
semantics to document the distinction. Ensure you reference the existing
decompress_action, its completed_proc.returncode check, and
decompress_action.log_file_path when forming the VerificationResult.fail()
return.
In `@integration-tests/tests/package_tests/utils/decompress.py`:
- Around line 40-44: The extraction_dir parameter on decompress_clp_package is
typed as Any but is always a Path; change its annotation to extraction_dir: Path
to match DecompressArgs and enable proper type checking, and update the same for
the other similar function that takes extraction_dir (the block referenced at
lines 60-63); after changing both, remove Any from the imports if it is no
longer used.
- Around line 67-76: The code creates a DecompressArgs instance and then mutates
its paths attribute; instead, pass paths into the DecompressArgs constructor
directly. Modify the block that builds the args variable (creating
DecompressArgs) to include the paths parameter when paths is truthy (so
DecompressArgs(..., paths=paths, ...)) and remove the subsequent if paths:
args.paths = paths mutation; this keeps creation atomic and idiomatic for the
DecompressArgs model.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: fe78face-e0ac-4d6c-a55d-68e7c2074457
📒 Files selected for processing (3)
integration-tests/tests/package_tests/utils/compress.pyintegration-tests/tests/package_tests/utils/decompress.pyintegration-tests/tests/utils/classes.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@integration-tests/tests/package_tests/utils/compress.py`:
- Around line 125-128: The current _verify_compress_action_structured_logs
function always returns VerificationResult.ok(), causing false positives; change
it to return an explicit failure until structured decompression verification
exists by replacing the ok() return with VerificationResult.fail("Structured
decompression verification not implemented") (or an equivalent
failure/gated-skip using VerificationResult) so the test fails/gates instead of
passing; update the return in _verify_compress_action_structured_logs to use
VerificationResult.fail(...) and include a short explanatory message.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5161e114-6164-4175-91dc-da2154ad90f4
📒 Files selected for processing (2)
integration-tests/tests/package_tests/utils/compress.pyintegration-tests/tests/package_tests/utils/decompress.py
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.
Description
This PR adds the action and verification infrastructure for testing package compression and decompression.
Checklist
breaking change.
Validation performed
Tested on development branch; all tests pass.
Summary by CodeRabbit
Tests
Chores