Skip to content

Conversation

@brandonschabell
Copy link
Owner

No description provided.

Copilot AI review requested due to automatic review settings October 13, 2025 17:09
Copy link

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 adds additional unit tests for URL constructor behavior and modifies the URL handling code to ensure consistent escape sequence cleanup across multi-argument construction patterns.

Key Changes

  • Enhanced URL constructor to handle multi-argument scenarios consistently across Python versions
  • Improved escape sequence cleanup to handle multiple encoding formats
  • Added comprehensive unit tests for constructor canonicalization and multi-argument joining

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
urlpath/_utils.py Enhanced cleanup_escapes function to handle multiple escape sequence formats
urlpath/_url.py Refactored URL constructor logic and improved escape cleanup in _combine_args method
tests/test_url.py Added new unit tests for constructor canonicalization and multi-argument semantics, removed webob conditional imports

import webob
except ImportError:
webob = None
import webob
Copy link

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

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

Importing webob unconditionally but the tests previously had conditional import logic. This will cause test failures when webob is not installed, breaking the optional dependency pattern.

Suggested change
import webob
try:
import webob
HAS_WEBOB = True
except ImportError:
HAS_WEBOB = False

Copilot uses AI. Check for mistakes.


@pytest.mark.skipif(webob is None, reason="webob not installed")
def test_webob() -> None:
Copy link

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

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

Removed @pytest.mark.skipif decorators for webob tests but kept unconditional webob import. These tests will fail when webob is not installed, as the skipif decorators were essential for optional dependency handling.

Copilot uses AI. Check for mistakes.


@pytest.mark.skipif(webob is None, reason="webob not installed")
def test_webob_jail() -> None:
Copy link

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

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

Removed @pytest.mark.skipif decorators for webob tests but kept unconditional webob import. These tests will fail when webob is not installed, as the skipif decorators were essential for optional dependency handling.

Copilot uses AI. Check for mistakes.
@brandonschabell brandonschabell merged commit e15116e into main Oct 13, 2025
7 checks passed
@brandonschabell brandonschabell deleted the add-tests branch October 13, 2025 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants