Intelligent test filtering if only test files are modified

[rzabarazesh]

This will make it so that if only some tests are modified, we will only run those tests in the CI.

Because jinja is hard to read, this is the python translation to make understanding it easier:

def intelligent_test_targeting(
    list_file_diff: str,  # Pipe-separated: "tests/engine/test_llm.py|vllm/config.py"
    step_dependencies: List[str],  # e.g., ["vllm/", "tests/engine", "tests/test_sequence"]
    step_commands: List[str],  # e.g., ["pytest -v -s engine test_sequence.py ..."]
    cov_enabled: bool = False
) -> str:
    """
    Generate optimized or default test command based on changed files.
    
    Returns:
        Command string to execute (optimized if possible, default otherwise)
    """
    
    # =========================================================================
    # STEP 1: Detect if ONLY test files changed (Lines 21-31)
    # =========================================================================
    files = [f for f in list_file_diff.split('|') if f]
    
    only_tests = True
    any_test = False
    changed_tests = []  # Relative to tests/ directory
    
    for file in files:
        # Check if file is a test: starts with "tests/", contains "/test_", ends with ".py"
        if file[:6] == 'tests/' and '/test_' in file and file[-3:] == '.py':
            any_test = True
            changed_tests.append(file[6:])  # Strip "tests/" prefix
        else:
            only_tests = False  # Found a non-test file
    
    tests_only = only_tests and any_test
    
    # If not tests_only, use default commands
    if not tests_only or not step_dependencies:
        return ' && '.join(step_commands)  # DEFAULT
    
    # =========================================================================
    # STEP 2: Match changed tests to step's test dependencies (Lines 48-74)
    # =========================================================================
    matched_targets = []
    
    for dep in step_dependencies:
        # Only process test dependencies
        if dep[:6] != 'tests/':
            continue
        
        dep_rel = dep[6:]  # Remove "tests/" prefix
        
        # Handle trailing slashes (e.g., "benchmarks/" vs "engine")
        if dep_rel[-1:] == '/':
            dep_dir_prefix = dep_rel  # Keep slash: "benchmarks/"
            dep_file_name = dep_rel[:-1] + '.py'  # "benchmarks.py"
        else:
            dep_dir_prefix = dep_rel + '/'  # Add slash: "engine/"
            dep_file_name = dep_rel + '.py'  # "test_sequence.py"
        
        # Check each changed test
        for t in changed_tests:
            prefix_len = len(dep_dir_prefix)
            
            # Directory match: "engine/test_llm.py" starts with "engine/"
            is_dir_match = (len(t) >= prefix_len and t[:prefix_len] == dep_dir_prefix)
            
            # File match: "test_sequence.py" == "test_sequence.py"
            is_file_match = (t == dep_file_name)
            
            if is_dir_match or is_file_match:
                matched_targets.append(t)
    
    # =========================================================================
    # STEP 3: Generate command (Lines 77-92)
    # =========================================================================
    if matched_targets:
        # OPTIMIZED: Run only matched tests
        if cov_enabled:
            return f"COVERAGE_FILE=.coverage pytest --cov=vllm {' '.join(matched_targets)}"
        else:
            return f"pytest -v -s {' '.join(matched_targets)}"
    else:
        # DEFAULT: Run original commands
        return ' && '.join(step_commands)

Testing

Test : Only engine test file modified -> Only running the new test
vllm-project/vllm@db06bd0
Build: https://buildkite.com/vllm/ci/builds/33083/steps/canvas?sid=0199a0cb-d268-42e4-b644-967f4e59a267

Test: Some test files and non-test files modified --> Behaving as before
https://github.com/vllm-project/vllm/pull/26036/files
Build: https://buildkite.com/vllm/ci/builds/33100/steps/canvas?sid=0199a13c-d6da-4781-8ea2-f968e48c1826

[seemethere]

I'll trust you that this works, because this is hurting my eyes to read.

[yeqcharlotte]

ehhh should we keep jinja as it is or looking for other options to configure pipelines? cc @khluu

[rzabarazesh]

ehhh should we keep jinja as it is or looking for other options to configure pipelines? cc @khluu

There was an attempt to refactor jinja and use python to generate the pipeline files but I believe it's no longer actively being worked on.

[khluu]

I feel really sorry for anyone who has to look at and work on this jinja file... we should've never had any complex logic in jinja in the first place.
And yes I had a project to convert this jinja template into a Python pipeline generator https://github.com/vllm-project/ci-infra/tree/main/buildkite/pipeline_generator that is not done yet but I don't have bandwidth now to pick it up.

[khluu]

@rzabarazesh Can you run a few builds to verify that it works?

[rzabarazesh]

@rzabarazesh Can you run a few builds to verify that it works?

I have. The builds are linked at the bottom of the first post

[yeqcharlotte]

LGTM!

[tdoublep]

This logic seems to ignore whether the test files that were changed are actually included in the original step_commands. This can lead to trying to run inappropriate tests within a given job (e.g., trying to run tests that need GPU within CPU-only CI jobs). This is currently blocking merging some PRs.

[rzabarazesh]

This logic seems to ignore whether the test files that were changed are actually included in the original step_commands. This can lead to trying to run inappropriate tests within a given job (e.g., trying to run tests that need GPU within CPU-only CI jobs). This is currently blocking merging some PRs.

I responded in your original PR