Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tests: Improved parametrization #964

Merged
merged 13 commits into from
Feb 17, 2025
Merged

Conversation

tony
Copy link
Member

@tony tony commented Feb 17, 2025

Summary

This PR converts all remaining pytest parametrized tests to use NamedTuple fixtures instead of raw tuples. This improves test readability, maintainability, and type safety.

Changes

Converted all parametrized tests to use NamedTuple fixtures:

test_cli.py

  • Added HelpTestFixture for help command tests
    • Converted test_help to use named parameters

test_convert.py

  • Added ConvertTestFixture for YAML conversion tests
    • Converted test_convert to use named parameters
  • Added ConvertJsonTestFixture for JSON conversion tests
    • Converted test_convert_json to use named parameters

test_freeze.py

  • Added FreezeTestFixture for freeze command tests
    • Converted test_freeze to use named parameters
  • Added FreezeOverwriteTestFixture for force overwrite tests
    • Converted test_freeze_overwrite to use named parameters

test_import.py

  • Added ImportTestFixture for basic import tests
    • Converted test_import to use named parameters
  • Added ImportTeamocilTestFixture for teamocil import tests
    • Converted test_import_teamocil to use named parameters
  • Added ImportTmuxinatorTestFixture for tmuxinator import tests
    • Converted test_import_tmuxinator to use named parameters

test_load.py

  • Added ZshAutotitleTestFixture for ZSH auto title warning tests
    • Converted test_load_zsh_autotitle_warning to use named parameters
  • Added LogFileTestFixture for log file tests
    • Converted test_load_log_file to use named parameters
  • Added PluginVersionTestFixture for plugin version tests
    • Converted test_load_plugins_version_fail_skip to use named parameters
    • Converted test_load_plugins_version_fail_no_skip to use named parameters
  • Added PluginMissingTestFixture for missing plugin tests
    • Converted test_load_plugins_plugin_missing to use named parameters

Each conversion includes:

  1. New NamedTuple class with docstring
  2. Test fixture list with descriptive test IDs
  3. Updated parametrize decorator to use NamedTuple fields
  4. Added test_id parameter to test function signature
  5. Improved test case documentation through fixture names and test IDs

Benefits

  1. Improved Type Safety: NamedTuples provide type hints for test parameters, making it easier to catch type-related issues
  2. Better Test Documentation: Each fixture class includes a docstring explaining its purpose
  3. More Maintainable Tests: Parameters are accessed by name rather than position, reducing the chance of errors when modifying tests
  4. Clearer Test IDs: Each test case has a descriptive ID, making test failures easier to understand
  5. Consistent Style: Aligns with existing NamedTuple fixtures used elsewhere in the codebase

Example

Before:

@pytest.mark.parametrize(
    "cli_args",
    [["--help"], ["-h"]],
)
def test_help(cli_args: list[str], ...):

After:

class HelpTestFixture(t.NamedTuple):
    """Test fixture for help command tests."""
    test_id: str
    cli_args: list[str]

HELP_TEST_FIXTURES = [
    HelpTestFixture(
        test_id="help_long_flag",
        cli_args=["--help"],
    ),
    HelpTestFixture(
        test_id="help_short_flag",
        cli_args=["-h"],
    ),
]

@pytest.mark.parametrize(
    list(HelpTestFixture._fields),
    HELP_TEST_FIXTURES,
    ids=[test.test_id for test in HELP_TEST_FIXTURES],
)
def test_help(test_id: str, cli_args: list[str], ...):

Testing

  • All existing tests continue to pass
  • No functional changes - this is purely a refactoring of test structure
  • Each test file has been verified to maintain the same test coverage

Related

Continues the work of standardizing test fixtures across the codebase, following the pattern established in files like test_builder.py and test_shell.py.

Summary by Sourcery

Refactor existing parametrized tests to use NamedTuple test fixtures, improving readability and maintainability. Add tests for missing scenarios like nonexistent targets and interactive shell in shell command tests. Parametrize workspace finder tests to cover various path scenarios and workspace load tests to cover different enter and sleep behaviors.

Tests:

  • Improve test parametrization by creating reusable test fixtures for shell command tests, teamocil, tmuxinator config import tests, and workspace load tests.
  • Add tests for missing scenarios like nonexistent targets and interactive shell in shell command tests.
  • Parametrize workspace finder tests to cover various path scenarios.
  • Parametrize workspace load tests to cover different enter and sleep behaviors.

Copy link

sourcery-ai bot commented Feb 17, 2025

Reviewer's Guide by Sourcery

This pull request improves test parameterization by converting test fixtures to use NamedTuple classes. This change enhances test readability, maintainability, and type safety across multiple test files.

No diagrams generated as the changes look simple and do not need a visual representation.

File-Level Changes

Change Details Files
Refactored CLI shell tests to use fixtures for improved readability and maintainability. The tests now cover missing scenarios such as nonexistent targets and interactive shell sessions.
  • Introduced CLIShellFixture, CLIShellTargetMissingFixture, and CLIShellInteractiveFixture NamedTuples to represent test cases.
  • Parametrized test_shell to use CLIShellFixture.
  • Added test_shell_target_missing to test scenarios with missing targets.
  • Added test_shell_interactive to test interactive shell scenarios.
  • Expanded test coverage to include cases with --pdb.
tests/cli/test_shell.py
Refactored workspace builder tests to use fixtures for better readability and maintainability. The tests now cover various path scenarios.
  • Introduced WorkspaceEnterFixture and WorkspaceSleepFixture NamedTuples to represent test cases.
  • Parametrized test_load_workspace_enter to use WorkspaceEnterFixture.
  • Parametrized test_load_workspace_sleep to use WorkspaceSleepFixture.
tests/workspace/test_builder.py
Refactored CLI load tests to use fixtures for improved readability and maintainability. The tests now cover zsh auto title warning, log file loading, and plugin loading scenarios.
  • Introduced ZshAutotitleTestFixture, LogFileTestFixture, PluginVersionTestFixture, and PluginMissingTestFixture NamedTuples to represent test cases.
  • Parametrized test_load_zsh_autotitle_warning to use ZshAutotitleTestFixture.
  • Parametrized test_load_log_file to use LogFileTestFixture.
  • Parametrized test_load_plugins_version_fail_skip and test_load_plugins_version_fail_no_skip to use PluginVersionTestFixture.
  • Parametrized test_load_plugins_plugin_missing to use PluginMissingTestFixture.
tests/cli/test_load.py
Refactored CLI import tests to use fixtures for improved readability and maintainability. The tests now cover teamocil and tmuxinator config import scenarios.
  • Introduced ImportTestFixture, ImportTeamocilTestFixture, and ImportTmuxinatorTestFixture NamedTuples to represent test cases.
  • Parametrized test_import to use ImportTestFixture.
  • Parametrized test_import_teamocil to use ImportTeamocilTestFixture.
  • Parametrized test_import_tmuxinator to use ImportTmuxinatorTestFixture.
tests/cli/test_import.py
Refactored teamocil config import tests to use fixtures for improved readability and maintainability. The tests now cover various config formats and options.
  • Introduced TeamocilConfigTestFixture and TeamocilMultiSessionTestFixture NamedTuples to represent test cases.
  • Parametrized test_config_to_dict to use TeamocilConfigTestFixture.
  • Parametrized test_multisession_config to use TeamocilMultiSessionTestFixture.
tests/workspace/test_import_teamocil.py
Refactored workspace finder tests to use fixtures for improved readability and maintainability. The tests now cover various path scenarios.
  • Introduced PureNameTestFixture NamedTuple to represent test cases.
  • Parametrized test_is_pure_name to use PureNameTestFixture.
tests/workspace/test_finder.py
Refactored CLI freeze tests to use fixtures for improved readability and maintainability. The tests now cover session freezing and overwrite scenarios.
  • Introduced FreezeTestFixture and FreezeOverwriteTestFixture NamedTuples to represent test cases.
  • Parametrized test_freeze to use FreezeTestFixture.
  • Parametrized test_freeze_overwrite to use FreezeOverwriteTestFixture.
tests/cli/test_freeze.py
Refactored CLI convert tests to use fixtures for improved readability and maintainability. The tests now cover different file extensions and options.
  • Introduced ConvertTestFixture and ConvertJsonTestFixture NamedTuples to represent test cases.
  • Parametrized test_convert to use ConvertTestFixture.
  • Parametrized test_convert_json to use ConvertJsonTestFixture.
tests/cli/test_convert.py
Refactored tmuxinator config import tests to use fixtures for improved readability and maintainability. The tests now cover different config formats and options.
  • Introduced TmuxinatorConfigTestFixture NamedTuple to represent test cases.
  • Parametrized test_config_to_dict to use TmuxinatorConfigTestFixture.
tests/workspace/test_import_tmuxinator.py
Refactored utility tests to use fixtures for improved readability and maintainability. The tests now cover isatty behavior verification.
  • Introduced TTYTestFixture NamedTuple to represent test cases.
  • Parametrized test_run_before_script_isatty to use TTYTestFixture.
tests/test_util.py
Refactored CLI tests to use fixtures for improved readability and maintainability. The tests now cover help command scenarios.
  • Introduced HelpTestFixture NamedTuple to represent test cases.
  • Parametrized test_help to use HelpTestFixture.
tests/cli/test_cli.py

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 your dashboard 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

Copy link

codecov bot commented Feb 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.06%. Comparing base (5fbc021) to head (25d7238).
Report is 14 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #964      +/-   ##
==========================================
- Coverage   73.11%   73.06%   -0.06%     
==========================================
  Files          26       26              
  Lines        1856     1856              
  Branches      352      352              
==========================================
- Hits         1357     1356       -1     
- Misses        395      396       +1     
  Partials      104      104              

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

Copy link

@sourcery-ai sourcery-ai bot 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:

  • The new test fixtures are a great way to organize the test data and make the tests more readable.
  • Consider adding a short description to each test fixture to explain its purpose.
Here's what I looked at during the review
  • 🟡 General issues: 1 issue found
  • 🟢 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.

docs/cli/shell.md Outdated Show resolved Hide resolved
@tony tony force-pushed the pytest-improved-parametrization branch 4 times, most recently from 3f418be to 82ee5cd Compare February 17, 2025 13:44
@tony tony force-pushed the pytest-improved-parametrization branch from 82ee5cd to 2ff09e3 Compare February 17, 2025 13:47
@tony
Copy link
Member Author

tony commented Feb 17, 2025

@sourcery-ai review

Copy link

@sourcery-ai sourcery-ai bot 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:

  • Consider adding a brief explanation of why NamedTuple was chosen over other alternatives like dataclasses.
  • The test IDs are very helpful, but ensure they are descriptive and follow a consistent naming convention.
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.

@tony tony force-pushed the pytest-improved-parametrization branch from 74c385a to cab166e Compare February 17, 2025 13:54
@tony tony force-pushed the pytest-improved-parametrization branch from afdc1a8 to 54e82c5 Compare February 17, 2025 14:01
@tony tony force-pushed the pytest-improved-parametrization branch from 026201e to 8d1a656 Compare February 17, 2025 14:12
@tony
Copy link
Member Author

tony commented Feb 17, 2025

@sourcery-ai review

Copy link

@sourcery-ai sourcery-ai bot 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 refactor - the tests are much easier to read now.
  • Consider adding a helper function to reduce the boilerplate when defining the test ID.
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.

@tony tony merged commit c556ec4 into master Feb 17, 2025
19 checks passed
@tony tony deleted the pytest-improved-parametrization branch February 17, 2025 14:33
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.

1 participant