Skip to content

Conversation

@aseembits93
Copy link
Contributor

@aseembits93 aseembits93 commented Jun 13, 2025

User description

TODO adding tests


PR Type

Enhancement


Description

  • Add transformer injecting request parameter

  • Import PositionProvider for transformer metadata

  • Apply AddRequestArgument before fixture modifier

  • Enhance disable_autouse function workflow


Changes walkthrough 📝

Relevant files
Enhancement
code_replacer.py
Add request argument transformer and reorder visits           

codeflash/code_utils/code_replacer.py

  • Added import of PositionProvider
  • Introduced AddRequestArgument transformer
  • Updated disable_autouse visit sequence
  • +32/-1   

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

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

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

    Metadata Usage

    The AddRequestArgument transformer declares PositionProvider as a metadata dependency, but the module isn't wrapped with a MetadataWrapper to supply metadata, which may lead to runtime errors.

    class AddRequestArgument(cst.CSTTransformer):
        METADATA_DEPENDENCIES = (PositionProvider,)
    
        def leave_FunctionDef(self, original_node: cst.FunctionDef, updated_node: cst.FunctionDef) -> cst.FunctionDef:  # noqa: ARG002
            args = updated_node.params.params
            arg_names = {arg.name.value for arg in args}
    Broad Transformer Scope

    AddRequestArgument is applied to all function definitions, potentially injecting request into functions where it's not intended; consider restricting it to fixture functions only.

    class AddRequestArgument(cst.CSTTransformer):
        METADATA_DEPENDENCIES = (PositionProvider,)
    
        def leave_FunctionDef(self, original_node: cst.FunctionDef, updated_node: cst.FunctionDef) -> cst.FunctionDef:  # noqa: ARG002
            args = updated_node.params.params
            arg_names = {arg.name.value for arg in args}
    
            # Skip if 'request' is already present
            if "request" in arg_names:
                return updated_node
    
            # Create a new 'request' param
            request_param = cst.Param(name=cst.Name("request"))
    
            # Add 'request' as the first argument (after 'self' or 'cls' if needed)
            if args:
                first_arg = args[0].name.value
                if first_arg in {"self", "cls"}:
                    new_params = [args[0], request_param] + list(args[1:])  # noqa: RUF005
                else:
                    new_params = [request_param] + list(args)  # noqa: RUF005
            else:
                new_params = [request_param]
    
            new_param_list = updated_node.params.with_changes(params=new_params)
            return updated_node.with_changes(params=new_param_list)

    @github-actions
    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Swap transformer visit order

    Apply the autouse fixture modifier first, then add the request parameter to its
    output. This ensures fixtures altered by AutouseFixtureModifier also receive the new
    argument.

    codeflash/code_utils/code_replacer.py [171-174]

     add_request_argument = AddRequestArgument()
     disable_autouse_fixture = AutouseFixtureModifier()
    -modified_module = module.visit(add_request_argument)
    -modified_module = modified_module.visit(disable_autouse_fixture)
    +modified_module = module.visit(disable_autouse_fixture)
    +modified_module = modified_module.visit(add_request_argument)
    Suggestion importance[1-10]: 8

    __

    Why: Applying the autouse modifier before adding the request parameter ensures all generated fixtures receive the new argument, addressing a potential functional bug.

    Medium
    General
    Limit transformer to fixtures

    Restrict adding a request parameter only to functions decorated as pytest fixtures.
    Early-return for non-fixtures to avoid polluting regular functions.

    codeflash/code_utils/code_replacer.py [44-45]

     def leave_FunctionDef(self, original_node: cst.FunctionDef, updated_node: cst.FunctionDef) -> cst.FunctionDef:
    +    # Only modify functions marked as pytest fixtures
    +    if not any(
    +        isinstance(deco.decorator, cst.Call)
    +        and getattr(deco.decorator.func, "attr", None) == "fixture"
    +        for deco in original_node.decorators
    +    ):
    +        return updated_node
         args = updated_node.params.params
    Suggestion importance[1-10]: 7

    __

    Why: Scoping the transformer to pytest fixtures prevents unintended modifications of regular functions and improves correctness without major changes.

    Medium

    misrasaurabh1
    misrasaurabh1 previously approved these changes Jun 13, 2025
    Copy link
    Contributor

    @misrasaurabh1 misrasaurabh1 left a comment

    Choose a reason for hiding this comment

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

    Hope this works with Galileo

    @aseembits93 aseembits93 merged commit 5b33d1d into main Jun 14, 2025
    16 checks passed
    @aseembits93 aseembits93 deleted the ignore-conftest branch June 16, 2025 19:14
    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