Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Improve test coverage#462

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
tony wants to merge128 commits intomaster
base:master
Choose a base branch
Loading
fromimprove-test-coverage
Open

Improve test coverage#462

tony wants to merge128 commits intomasterfromimprove-test-coverage

Conversation

@tony
Copy link
Member

@tonytony commentedMar 8, 2025•
edited by sourcery-aibot
Loading

Summary by Sourcery

Documentation:

  • Adds a comprehensive document describing the VCSPull project, its architecture, configuration, and usage.

@sourcery-ai
Copy link

sourcery-aibot commentedMar 8, 2025•
edited
Loading

Reviewer's Guide by Sourcery

This pull request adds a comprehensive 'about.md' document to the 'notes' directory. This document provides a detailed overview of the VCSPull project, covering its purpose, architecture, configuration, codebase structure, development practices, tooling, and usage.

Class Diagram for VCSPull Configuration

classDiagram    class ConfigFile {        +str path        +dict repos    }    class Repository {        +str url        +dict remotes    }    class VCSClient {        +str vcs_type        +str url        +sync()    }    ConfigFile -- Repository : contains    Repository -- VCSClient : uses
Loading

File-Level Changes

ChangeDetailsFiles
Added a comprehensive project analysis document.
  • Introduced a detailed overview of the VCSPull project.
  • Described the core purpose of VCSPull, including simplifying repository management and enabling declarative configuration.
  • Outlined the configuration-driven architecture and key design patterns such as Factory, Command, Facade, and Template Method.
  • Explained the YAML/JSON configuration format with examples.
  • Detailed the codebase structure, including core components like Configuration Management, CLI Interface, and Type System.
  • Listed the project's dependencies, including libvcs, PyYAML, and colorama.
  • Summarized the development practices, such as strong type hints, comprehensive test coverage, and modern Python features.
  • Described the project's tooling, including uv, Ruff, Mypy, and Pytest.
  • Documented the configuration file locations and usage patterns.
  • Provided insights into the project's evolution and architecture.
notes/2025-03-08 - about.md
Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment@sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with@sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write@sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write@sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment@sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment@sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment@sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment@sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!
  • Generate a plan of action for an issue: Comment@sourcery-ai plan on
    an issue to generate a plan of action for it.

Customizing Your Experience

Access yourdashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@codecov
Copy link

codecovbot commentedMar 8, 2025•
edited
Loading

Codecov Report

Attention: Patch coverage is22.46521% with780 lines in your changes missing coverage. Please review.

Project coverage is 23.92%. Comparing base(924ae0f) to head(3f9e9d4).

Files with missing linesPatch %Lines
src/vcspull/operations.py8.83%227 Missing⚠️
src/vcspull/cli/commands.py10.71%200 Missing⚠️
src/vcspull/vcs/mercurial.py17.74%102 Missing⚠️
src/vcspull/vcs/git.py18.33%98 Missing⚠️
src/vcspull/vcs/svn.py21.35%81 Missing⚠️
src/vcspull/vcs/base.py36.73%31 Missing⚠️
src/vcspull/config/loader.py68.42%19 Missing and 5 partials⚠️
src/vcspull/_internal/logger.py21.05%15 Missing⚠️
src/vcspull/config/models.py95.23%2 Missing⚠️
Additional details and impacted files
@@             Coverage Diff             @@##           master     #462       +/-   ##===========================================- Coverage   78.98%   23.92%   -55.07%===========================================  Files           8       10        +2       Lines         414     1045      +631       Branches       85      150       +65     ===========================================- Hits          327      250       -77- Misses         52      789      +737+ Partials       35        6       -29

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report?Share it here.

🚀 New features to boost your workflow:
  • ❄Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link

@sourcery-aisourcery-aibot left a comment

Choose a reason for hiding this comment

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

Hey@tony - I've reviewed your changes - here's some feedback:

Overall Comments:

  • This is a great overview of the project, but it's unclear how it improves test coverage as the title suggests.
Here's what I looked at during the review
  • 🟢General issues: all looks good
  • 🟢Security: all looks good
  • 🟢Testing: all looks good
  • 🟢Complexity: all looks good
  • 🟢Documentation: all looks good
Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

sourcery-ai[bot] reacted with thumbs up emojisourcery-ai[bot] reacted with thumbs down emoji
@tonytonyforce-pushed theimprove-test-coverage branch 6 times, most recently from45d442a to5be857cCompareMarch 8, 2025 22:38
@tonytonyforce-pushed themaster branch 2 times, most recently from02b87ec to3e3654bCompareMarch 8, 2025 22:52
@tonytonyforce-pushed theimprove-test-coverage branch from5be857c toac5af18CompareMarch 8, 2025 22:53
@tonytonyforce-pushed theimprove-test-coverage branch 9 times, most recently from126baa7 to06fdc1fCompareMarch 15, 2025 11:27
# Infer VCS from URL if not already set
if "vcs" not in repo_data and "url" in repo_data:
url = repo_data["url"]
if "github.com" in url or url.endswith(".git"):

Check failure

Code scanning / CodeQL

Incomplete URL substring sanitization High

The string
github.com
Loading
may be at an arbitrary position in the sanitized URL.

Copilot Autofix

AI 9 months ago

To fix the problem, we need to replace the substring check with a more robust method that parses the URL and checks the hostname. Specifically, we should use theurlparse function from theurllib.parse module to extract the hostname and then check if it matches the expected domain.

  1. Import theurlparse function from theurllib.parse module.
  2. Replace the substring check with a parsed URL check.
  3. Ensure that the check handles arbitrary subdomain sequences correctly.
Suggested changeset1
src/vcspull/config/migration.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git applydiff --git a/src/vcspull/config/migration.py b/src/vcspull/config/migration.py--- a/src/vcspull/config/migration.py+++ b/src/vcspull/config/migration.py@@ -13,3 +13,3 @@ from typing import Any, Optional-+from urllib.parse import urlparse import yaml@@ -222,5 +222,6 @@                     url = repo_data["url"]-                    if "github.com" in url or url.endswith(".git"):+                    parsed_url = urlparse(url)+                    if parsed_url.hostname and (parsed_url.hostname == "github.com" or url.endswith(".git")):                         repo_data["vcs"] = "git"-                    elif "bitbucket.org" in url and not url.endswith(".git"):+                    elif parsed_url.hostname and parsed_url.hostname == "bitbucket.org" and not url.endswith(".git"):                         repo_data["vcs"] = "hg"EOF
@@ -13,3 +13,3 @@
fromtypingimportAny,Optional

fromurllib.parseimporturlparse
importyaml
@@ -222,5 +222,6 @@
url=repo_data["url"]
if"github.com"inurlorurl.endswith(".git"):
parsed_url=urlparse(url)
ifparsed_url.hostnameand (parsed_url.hostname=="github.com"orurl.endswith(".git")):
repo_data["vcs"]="git"
elif"bitbucket.org"inurlandnoturl.endswith(".git"):
elifparsed_url.hostnameandparsed_url.hostname=="bitbucket.org"andnoturl.endswith(".git"):
repo_data["vcs"]="hg"
url = repo_data["url"]
if "github.com" in url or url.endswith(".git"):
repo_data["vcs"] = "git"
elif "bitbucket.org" in url and not url.endswith(".git"):

Check failure

Code scanning / CodeQL

Incomplete URL substring sanitization High

The string
bitbucket.org
Loading
may be at an arbitrary position in the sanitized URL.

Copilot Autofix

AI 9 months ago

To fix the problem, we need to ensure that the URL is properly parsed and validated to confirm that the host is exactly "bitbucket.org" and not just a substring within the URL. We can use theurlparse function from theurllib.parse module to parse the URL and then check the hostname.

  • Parse the URL usingurlparse.
  • Extract the hostname from the parsed URL.
  • Check if the hostname is exactly "bitbucket.org".

This change should be made in themigrate_v1_to_v2 function, specifically around the lines where the URL is being checked for "bitbucket.org".

Suggested changeset1
src/vcspull/config/migration.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git applydiff --git a/src/vcspull/config/migration.py b/src/vcspull/config/migration.py--- a/src/vcspull/config/migration.py+++ b/src/vcspull/config/migration.py@@ -222,5 +222,6 @@                     url = repo_data["url"]-                    if "github.com" in url or url.endswith(".git"):+                    parsed_url = urlparse(url)+                    if "github.com" in parsed_url.hostname or url.endswith(".git"):                         repo_data["vcs"] = "git"-                    elif "bitbucket.org" in url and not url.endswith(".git"):+                    elif parsed_url.hostname == "bitbucket.org" and not url.endswith(".git"):                         repo_data["vcs"] = "hg"EOF
@@ -222,5 +222,6 @@
url=repo_data["url"]
if"github.com"inurlorurl.endswith(".git"):
parsed_url=urlparse(url)
if"github.com"inparsed_url.hostnameorurl.endswith(".git"):
repo_data["vcs"]="git"
elif"bitbucket.org"inurlandnoturl.endswith(".git"):
elifparsed_url.hostname=="bitbucket.org"andnoturl.endswith(".git"):
repo_data["vcs"]="hg"
@tonytonyforce-pushed theimprove-test-coverage branch 3 times, most recently from982b920 to87e8c24CompareMarch 29, 2025 10:39
@tonytonyforce-pushed theimprove-test-coverage branch from87e8c24 to98e5e92CompareApril 5, 2025 08:54
@tonytonyforce-pushed theimprove-test-coverage branch from98e5e92 tof479f5cCompareApril 12, 2025 12:16
tony added28 commitsApril 19, 2025 05:42
…odelswhy: Enhance test coverage and verification of configuration models through property-based testing,ensuring models behave correctly with a wide variety of inputs beyond specific examples.what:- Implement property-based testing using Hypothesis for configuration models- Create comprehensive test strategies for generating valid URLs, paths, and model instances- Add tests verifying serialization roundtrips and invariant properties- Ensure tests verify Repository, Settings, VCSPullConfig, LockFile, and LockedRepository models- Fix type annotations and linting issues in test files- Add Hypothesis dependency to development dependenciesrefs: Addresses "Property-Based Testing" item from TODO.md
…n loaderwhy: Enhance test coverage and reliability of the configuration system by implementingproperty-based testing with Hypothesis and comprehensive integration tests.what:- Created property-based tests for configuration loading, saving, and include resolution- Added test generators for repository URLs, paths, and configuration objects- Implemented integration tests for complete configuration workflow- Fixed circular include detection in resolve_includes to prevent infinite recursion- Added proper tracking of processed paths to avoid duplicated processing- Ensured all code follows project style guidelines and has proper type annotations- Improved test reliability with proper temporary file and directory handlingrefs: Completes "Property-Based Testing" section in notes/TODO.md
why: Facilitate user transition from old nested config format to new Pydantic v2 formatwhat:- Implement migration module to detect and convert configuration versions- Add CLI command with dry-run, backup, and color output options- Create comprehensive test suite with property-based testing- Write detailed migration guide for usersSee also: notes/proposals/01-config-format-structure.md
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

1 more reviewer

@sourcery-aisourcery-ai[bot]sourcery-ai[bot] left review comments

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tony
[8]ページ先頭
©2009-2025 Movatter.jp