Update for kwargs on artifact upload#91
Conversation
Reviewer's GuideThis PR refactors the artifact upload flow to use explicit keyword arguments for filename and file, updates test cases to capture and assert on those kwargs instead of positional args, and expands project dependencies in pyproject.toml to support new test tooling. Sequence diagram for updated artifact upload with keyword argumentssequenceDiagram
participant Sender
participant ArtifactAPI
participant Handler
Sender->>Handler: Get prepared_data
Sender->>ArtifactAPI: upload_artifact(filename=filename, file=prepared_data, **kwargs)
ArtifactAPI-->>Sender: Response
Class diagram for updated _upload_artifact method in SenderclassDiagram
class Sender {
_upload_artifact(filename, handler, **kwargs)
}
class Handler {
prepared_data
}
class ArtifactAPI {
upload_artifact(filename, file, **kwargs)
}
Sender --> Handler : uses
Sender --> ArtifactAPI : calls
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Pull Request Overview
This PR updates the artifact upload functionality to use keyword arguments instead of positional arguments, addressing compatibility issues with API changes. The changes align with corresponding updates in the ibutsu-server and ibutsu-client-python repositories.
- Modified artifact upload method to pass
filenameandfileas keyword arguments - Added type ignore comments to suppress type checking warnings
- Updated test dependencies in pyproject.toml to include explicit testing packages
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/pytest_ibutsu/sender.py | Updated artifact upload to use kwargs and added type ignore comments |
| pyproject.toml | Replaced generic test dependency with explicit testing packages |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
d9a853b to
a23dc3b
Compare
a23dc3b to
b10e8a9
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #91 +/- ##
=======================================
Coverage ? 91.63%
=======================================
Files ? 6
Lines ? 992
Branches ? 149
=======================================
Hits ? 909
Misses ? 51
Partials ? 32
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `tests/test_sender.py:481-485` </location>
<code_context>
- assert args[1] == "none_data.txt"
+ assert kwargs["filename"] == "none_data.txt"
# None gets converted to "None" string
assert sender._captured_data_content() == "None"
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding a test for uploading artifact with empty bytes.
A test for uploading empty bytes (b'') is missing. Please add one to ensure correct handling.
```suggestion
assert kwargs["filename"] == "empty.txt"
# Verify the data content was captured correctly - empty string passed as string
assert sender._captured_data_content() == "" # Empty string passed directly
sender._make_call.assert_called_once()
args, kwargs = sender._make_call.call_args
assert args[0] == sender.artifact_api.upload_artifact
assert kwargs["filename"] == "none_data.txt"
# None gets converted to "None" string
def test_upload_artifact_with_empty_bytes(sender):
"""
Test uploading an artifact with empty bytes (b'').
"""
captured_data_content = None
def capture_data(*args, **kwargs):
nonlocal captured_data_content
captured_data_content = kwargs.get("file", captured_data_content)
sender._make_call = Mock(side_effect=capture_data)
sender._captured_data_content = lambda: captured_data_content
sender.upload_artifact(filename="empty_bytes.txt", file=b"")
sender._make_call.assert_called_once()
args, kwargs = sender._make_call.call_args
assert args[0] == sender.artifact_api.upload_artifact
assert kwargs["filename"] == "empty_bytes.txt"
# Verify the data content was captured correctly - empty bytes passed as empty string
assert sender._captured_data_content() == b"" or sender._captured_data_content() == "" # Accept both representations
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Pairs with:
ibutsu/ibutsu-server#735
ibutsu/ibutsu-client-python#38
Summary by Sourcery
Use keyword arguments for artifact upload in sender.py and adjust tests and dependencies accordingly
Enhancements:
Build:
Tests: