Skip to content

prevent interference by other pytest plugins #61

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

Merged
merged 3 commits into from
Mar 20, 2025
Merged

Conversation

alvin-r
Copy link
Contributor

@alvin-r alvin-r commented Mar 20, 2025

User description

Two ways:

  1. Prevent discovery of tests with benchmark fixtures during test discovery.
  • This is necessary for any plugin that uses fixtures. If you disable it, the fixtures will throw an error.
  1. Add a blocklist of plugins
  • when running test, simply use -p no: {pluginname} to prevent the plugin from being used.

PR Type

  • Enhancement
  • Tests

Description

  • Added hook to skip tests with benchmark fixtures.

  • Injected plugin blocklist flags in test runner commands.

  • Created unit test to verify benchmark test exclusion.


Changes walkthrough 📝

Relevant files
Enhancement
pytest_new_process_discovery.py
Add hook to skip benchmark tests                                                 

codeflash/discovery/pytest_new_process_discovery.py

  • Introduced pytest_collection_modifyitems hook.
  • Applies skip marker for benchmark tests.
  • Adjusted pytest.main command parameters.
  • +6/-1     
    test_runner.py
    Integrate plugin blocklist into test runner                           

    codeflash/verification/test_runner.py

  • Defined constants for plugin blocklists.
  • Added blocklist args to test subprocess commands.
  • Improved test execution filtering.
  • +9/-3     
    Tests
    test_unit_test_discovery.py
    Add unit test for benchmark exclusion                                       

    tests/test_unit_test_discovery.py

  • Added new test for benchmark test discovery.
  • Confirms benchmark tests are skipped.
  • Validates correct test function discovery.
  • +42/-0   

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Fixture Filtering

    Verify that the new marker addition in the pytest_collection_modifyitems hook correctly identifies all benchmark fixture cases. Consider edge cases such as partial matches or case sensitivity.

    def pytest_collection_modifyitems(config, items):
        skip_benchmark = pytest.mark.skip(reason="Skipping benchmark tests")
        for item in items:
            if "benchmark" in item.fixturenames:
                item.add_marker(skip_benchmark)
    Plugin Blocklist Integration

    Review the concatenation of blocklist arguments with other pytest command-line arguments to ensure that disabling of plugins works reliably and does not conflict with existing options.

        blocklist_args = [f"-p no:{plugin}" for plugin in BEHAVIORAL_BLOCKLISTED_PLUGINS if plugin != "cov"]
        results = execute_test_subprocess(
            coverage_cmd + common_pytest_args + blocklist_args + result_args + test_files, cwd=cwd, env=pytest_test_env, timeout=600
        )
        logger.debug(
            f"Result return code: {results.returncode}, "
            f"{'Result stderr:' + str(results.stderr) if results.stderr else ''}"
        )
    else:
        blocklist_args = [f"-p no:{plugin}" for plugin in BEHAVIORAL_BLOCKLISTED_PLUGINS]
        results = execute_test_subprocess(
            pytest_cmd_list + common_pytest_args + blocklist_args + result_args + test_files,
            cwd=cwd,

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Fix method signature

    Add the missing self parameter to the hook method so it adheres to the expected
    instance method signature.

    codeflash/discovery/pytest_new_process_discovery.py [19-23]

    -def pytest_collection_modifyitems(config, items):
    +def pytest_collection_modifyitems(self, config, items):
         skip_benchmark = pytest.mark.skip(reason="Skipping benchmark tests")
         for item in items:
             if "benchmark" in item.fixturenames:
                 item.add_marker(skip_benchmark)
    Suggestion importance[1-10]: 9

    __

    Why: The suggestion correctly identifies that the hook method should include the 'self' parameter to be consistent with its class-based implementation, addressing a likely runtime issue.

    High

    @alvin-r alvin-r requested review from misrasaurabh1 and KRRT7 March 20, 2025 18:58
    KRRT7
    KRRT7 previously approved these changes Mar 20, 2025
    @alvin-r alvin-r enabled auto-merge March 20, 2025 22:19
    @alvin-r alvin-r merged commit 7942de0 into main Mar 20, 2025
    15 checks passed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    3 participants