Skip to content

Conversation

@KRRT7
Copy link
Contributor

@KRRT7 KRRT7 commented Jun 7, 2025

PR Type

Enhancement, Tests


Description

  • Introduce AST ImportAnalyzer for test file import analysis

  • Filter test files via imports before Jedi processing

  • Refactor discovery to use filtering and return test count

  • Update code and expand tests for import-filter functionality


Changes walkthrough 📝

Relevant files
Enhancement
6 files
discover_unit_tests.py
Introduce AST import analysis and test filtering                 
+226/-87
functions_to_optimize.py
Update discover_unit_tests signature usage                             
+1/-1     
function_optimizer.py
Accept set of tests in FunctionOptimizer                                 
+12/-7   
optimizer.py
Adapt optimizer to new test discovery signature                   
+4/-3     
create_pr.py
Support set-based test structures in PR creation                 
+1/-1     
concolic_testing.py
Update concolic tests discovery signature                               
+2/-3     
Formatting
1 files
test_static_analysis.py
Fix test import formatting                                                             
+1/-1     
Tests
1 files
test_unit_test_discovery.py
Add import-filtering unit tests and signature updates       
+471/-44

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

    github-actions bot commented Jun 7, 2025

    PR Reviewer Guide 🔍

    (Review updated until commit fd06dca)

    Here are some key observations to aid the review process:

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

    Missing Logger Import

    The new import analysis and filter functions (analyze_imports_in_test_file, filter_test_files_by_imports) use logger.debug and logger but there’s no visible import or definition of logger in this file. Ensure logger is imported or defined to avoid runtime errors.

        return False, set()  # noqa: TRY300
    
    except (SyntaxError, UnicodeDecodeError, OSError) as e:
        logger.debug(f"Failed to analyze imports in {test_file_path}: {e}")
        return True, set()
    Star Import Handling

    Wildcard imports (from module import *) are skipped and lead to should_process=False, filtering out tests that may actually reference target functions via *. Consider treating star imports as potential matches and returning True for Jedi processing instead of dropping them.

    if alias.name == "*":
        continue
    imported_name = alias.asname if alias.asname else alias.name
    Error Fallback Behavior

    On syntax or read errors, analyze_imports_in_test_file returns (True, set()), causing all unparseable files to be processed by Jedi with no identified targets. You may want to skip such files or log a warning to avoid wasted processing.

        return False, set()  # noqa: TRY300
    
    except (SyntaxError, UnicodeDecodeError, OSError) as e:
        logger.debug(f"Failed to analyze imports in {test_file_path}: {e}")
        return True, set()

    @github-actions
    Copy link

    github-actions bot commented Jun 7, 2025

    PR Code Suggestions ✨

    Latest suggestions up to fd06dca
    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Track real module names correctly

    Ensure that imported_modules always tracks the original module name and
    imported_names tracks the alias if present. This prevents mismatches when an import
    is aliased.

    codeflash/discovery/discover_unit_tests.py [153-159]

     def visit_Import(self, node: ast.Import) -> None:
         for alias in node.names:
    -        module_name = alias.asname if alias.asname else alias.name
    -        self.imported_modules.add(module_name)
    -        self.imported_names.add(module_name)
    +        # track the real module and the local alias separately
    +        self.imported_modules.add(alias.name)
    +        imported_name = alias.asname if alias.asname else alias.name
    +        self.imported_names.add(imported_name)
         self.generic_visit(node)
    Suggestion importance[1-10]: 7

    __

    Why: The change cleanly separates original module names from aliases in visit_Import, improving import detection accuracy.

    Medium
    Detect aliased function imports

    Also consider when a function is imported under an alias—check imported_name against
    function_names_to_find so aliased imports still register target functions.

    codeflash/discovery/discover_unit_tests.py [161-178]

     def visit_ImportFrom(self, node: ast.ImportFrom) -> None:
         for alias in node.names:
             if alias.name == "*":
                 continue
             imported_name = alias.asname if alias.asname else alias.name
             self.imported_names.add(imported_name)
    +        # handle direct and aliased imports of target functions
             if alias.name in self.function_names_to_find:
                 self.found_target_functions.add(alias.name)
    -        # Check for qualified name matches
    +        if imported_name in self.function_names_to_find:
    +            self.found_target_functions.add(imported_name)
             if node.module:
                 qualified_name = f"{node.module}.{alias.name}"
                 if qualified_name in self.function_names_to_find:
                     self.found_target_functions.add(qualified_name)
         self.generic_visit(node)
    Suggestion importance[1-10]: 7

    __

    Why: Adding a check for imported_name ensures aliased functions are registered in found_target_functions, fixing missing matches.

    Medium

    Previous suggestions

    Suggestions up to commit 236f14c
    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Fix undefined variable reference

    The variable file_to_funcs_to_optimize is not defined in this scope, causing a
    NameError at runtime. Replace it with the correct attribute (likely
    self.file_to_funcs_to_optimize or whatever member holds the mapping) so it
    references a valid class member.

    codeflash/optimization/optimizer.py [166-168]

     all_functions_to_optimize = [
    -    func for funcs_list in file_to_funcs_to_optimize.values() for func in funcs_list
    +    func for funcs_list in self.file_to_funcs_to_optimize.values() for func in funcs_list
     ]
    Suggestion importance[1-10]: 9

    __

    Why: The variable file_to_funcs_to_optimize is undefined in run, causing a NameError; using self.file_to_funcs_to_optimize fixes this critical bug.

    High

    @KRRT7 KRRT7 force-pushed the filter-tests-for-pr-test-discovery branch 2 times, most recently from b2ab78c to 87f7528 Compare June 7, 2025 08:33
    pre-commit cleanup
    
    guard path
    
    fix tests & remove goto cache
    
    remove TestsCache usage
    
    moved the num_discovered_tests calculation inside the
    discover_unit_tests
    
    dict directly by using a set in discover_unit_tests
    efficient passes
    @KRRT7 KRRT7 force-pushed the filter-tests-for-pr-test-discovery branch from 87f7528 to 918dd68 Compare June 7, 2025 18:30
    @KRRT7 KRRT7 marked this pull request as ready for review June 8, 2025 02:39
    @github-actions
    Copy link

    github-actions bot commented Jun 8, 2025

    Persistent review updated to latest commit fd06dca

    @misrasaurabh1 misrasaurabh1 changed the title Claude code test discovery in GHA optimization Test discovery in GHA optimization by pre-filtering test files Jun 8, 2025
    if node.module:
    qualified_name = f"{node.module}.{alias.name}"
    if qualified_name in self.function_names_to_find:
    self.found_target_functions.add(qualified_name)
    Copy link
    Contributor

    Choose a reason for hiding this comment

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

    when the first time function found in a file, then we skip processing the rest of the file. return True

    project_root_path = cfg.project_root_path
    test_framework = cfg.test_framework

    if functions_to_optimize:
    Copy link
    Contributor

    Choose a reason for hiding this comment

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

    shouldn't this always be True?

    Copy link
    Contributor Author

    Choose a reason for hiding this comment

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

    @misrasaurabh1 checked it out, in our previous implementation, discover_unit_tests was being called without the list of functions to optimize, so it was a backwards compatible change, going to update it to reflect the new behavior entirely.

    @misrasaurabh1 misrasaurabh1 merged commit 702bee2 into main Jun 8, 2025
    17 checks passed
    @KRRT7 KRRT7 deleted the filter-tests-for-pr-test-discovery branch June 8, 2025 04:46
    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