Skip to content

Conversation

SAKURA-CAT
Copy link
Member

Improves host string processing in save_key by correctly removing trailing '/api' and whitespace. Adds unit tests to verify correct host handling and extends HostFormatter test coverage.

Thanks to https://github.com/kingdee for the feedback.

@SAKURA-CAT SAKURA-CAT requested review from Zeyi-Lin and Copilot July 3, 2025 03:23
@SAKURA-CAT SAKURA-CAT self-assigned this Jul 3, 2025
@SAKURA-CAT SAKURA-CAT added the 🐛 bug Something isn't working label Jul 3, 2025
Improves host string processing in save_key by correctly removing trailing '/api' and whitespace. Adds unit tests to verify correct host handling and extends HostFormatter test coverage.
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refines how save_key normalizes the host string by removing trailing whitespace and an /api suffix, and it adds unit tests to validate this behavior and extend coverage for HostFormatter.

  • Update save_key to strip whitespace and remove an /api suffix from the host argument.
  • Add a test_host to verify correct host handling in .netrc and extend HostFormatter tests.
  • Ensure existing HostFormatter tests include a plain domain scenario.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
test/unit/test_package.py New test_host case for save_key and added HostFormatter assertion
swanlab/package.py save_key now uses rstrip() and conditional suffix removal
Comments suppressed due to low confidence (2)

swanlab/package.py:199

  • Add tests for hosts ending with "/api/" (with a trailing slash) and for cases where /api is followed by whitespace to ensure save_key strips these variations correctly.
    if host.endswith("/api"):

swanlab/package.py:187

  • [nitpick] Update the save_key docstring to note that trailing whitespace is trimmed and any /api suffix will be removed from the provided host.
def save_key(username: str, password: str, host: str = None) -> bool:

@SAKURA-CAT SAKURA-CAT marked this pull request as ready for review July 3, 2025 03:26
@SAKURA-CAT SAKURA-CAT merged commit c37e9ce into main Jul 3, 2025
5 checks passed
@SAKURA-CAT SAKURA-CAT deleted the fix/save branch July 3, 2025 06:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants