Skip to content

Conversation

@KRRT7
Copy link
Contributor

@KRRT7 KRRT7 commented Mar 14, 2025

User description

Jedi struggles with compiled modules, it's fine if we skip them.


PR Type

  • Bug fix
  • Enhancement

Description

  • Wrap jedi.Script call within try/except block.

  • Log debug message for script creation failures.

  • Skip processing test files that cause exceptions.

  • Remove obsolete test utility file.


Changes walkthrough 📝

Relevant files
Error handling
discover_unit_tests.py
Introduce error handling for jedi script creation               

codeflash/discovery/discover_unit_tests.py

  • Wrapped jedi.Script creation in try/except.
  • Added debug logging for exceptions.
  • Continues on exception to skip problematic files.
  • +13/-9   
    Other
    testbench.py
    Delete obsolete testbench file                                                     

    testbench.py

    • Fully removed obsolete testing file.
    +0/-54   

    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

    Exception Handling

    The PR introduces a broad try/except block that may hide unexpected errors. It is recommended to refine the exception handling to only catch known exceptions and add sufficient logging to aid debugging.

    try:
        script = jedi.Script(path=test_file, project=jedi_project)
        test_functions = set()
    
        all_names = script.get_names(all_scopes=True, references=True)
        all_defs = script.get_names(all_scopes=True, definitions=True)
        all_names_top = script.get_names(all_scopes=True)
    
        top_level_functions = {name.name: name for name in all_names_top if name.type == "function"}
        top_level_classes = {name.name: name for name in all_names_top if name.type == "class"}
    except Exception as e:
        logger.debug(f"Failed to get jedi script for {test_file}: {e}")
        continue

    @github-actions
    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Narrow try block scope

    Narrow the try block scope to isolate the untrusted initialization call while
    ensuring that function and class extraction errors are handled distinctly.

    codeflash/discovery/discover_unit_tests.py [189-201]

     try:
         script = jedi.Script(path=test_file, project=jedi_project)
    -    test_functions = set()
    +except Exception as e:
    +    logger.debug(f"Failed to initialize jedi Script for {test_file}: {e}")
    +    continue
    +test_functions = set()
    +all_names = script.get_names(all_scopes=True, references=True)
    +all_defs = script.get_names(all_scopes=True, definitions=True)
    +all_names_top = script.get_names(all_scopes=True)
    +top_level_functions = {name.name: name for name in all_names_top if name.type == "function"}
    +top_level_classes = {name.name: name for name in all_names_top if name.type == "class"}
     
    -    all_names = script.get_names(all_scopes=True, references=True)
    -    all_defs = script.get_names(all_scopes=True, definitions=True)
    -    all_names_top = script.get_names(all_scopes=True)
    -
    -    top_level_functions = {name.name: name for name in all_names_top if name.type == "function"}
    -    top_level_classes = {name.name: name for name in all_names_top if name.type == "class"}
    -except Exception as e:
    -    logger.debug(f"Failed to get jedi script for {test_file}: {e}")
    -    continue
    -
    Suggestion importance[1-10]: 6

    __

    Why: The suggestion is relevant as it refines the error handling by limiting the try block to the initialization of the jedi.Script, which can help isolate errors. However, it only offers a moderate improvement in functionality and might expose exceptions from subsequent operations without extra handling.

    Low

    @KRRT7 KRRT7 merged commit 0f81492 into main Mar 14, 2025
    15 checks passed
    @KRRT7 KRRT7 deleted the test-processing-fix branch March 14, 2025 05:13
    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