Skip to content

Conversation

KRRT7
Copy link
Contributor

@KRRT7 KRRT7 commented Oct 7, 2025

User description

this will prevent E2E running until the normal unit tests have finished running


PR Type

Enhancement, Other


Description

  • Gate E2E workflows on unit test success

  • Replace PR triggers with workflow_run

  • Add success condition to E2E jobs

  • Standardize unit test workflow name


Diagram Walkthrough

flowchart LR
  UT["Unit Tests workflow"] -- "completed: success" --> E2E1["E2E - Async"]
  UT -- "completed: success" --> E2E2["E2E - Bubble Sort Benchmark"]
  UT -- "completed: success" --> E2E3["E2E - Bubble Sort Pytest (No Git)"]
  UT -- "completed: success" --> E2E4["E2E - Bubble Sort Unittest"]
  UT -- "completed: success" --> E2E5["Coverage E2E"]
  UT -- "completed: success" --> E2E6["E2E - Futurehouse Structure"]
  UT -- "completed: success" --> E2E7["E2E - Init Optimization"]
  UT -- "completed: success" --> E2E8["E2E - Topological Sort"]
  UT -- "completed: success" --> E2E9["E2E - Tracer Replay"]
Loading

File Walkthrough

Relevant files
Configuration changes
10 files
e2e-async.yaml
Trigger E2E only after successful unit tests                         
+5/-4     
e2e-bubblesort-benchmark.yaml
Gate benchmark E2E on unit test success                                   
+5/-4     
e2e-bubblesort-pytest-nogit.yaml
Gate pytest-no-git E2E on unit tests                                         
+5/-4     
e2e-bubblesort-unittest.yaml
Gate unittest E2E on unit test success                                     
+5/-4     
e2e-coverage-optimization.yaml
Gate coverage E2E on unit test success                                     
+5/-4     
e2e-futurehouse-structure.yaml
Gate futurehouse E2E on unit test success                               
+5/-4     
e2e-init-optimization.yaml
Gate init optimization E2E on unit test success                   
+5/-4     
e2e-topological-sort.yaml
Gate topological sort E2E on unit test success                     
+5/-4     
e2e-tracer-replay.yaml
Gate tracer replay E2E on unit test success                           
+5/-4     
unit-tests.yaml
Rename workflow to 'Unit Tests' for dependency                     
+1/-2     

@github-actions github-actions bot added workflow-modified This PR modifies GitHub Actions workflows Review effort 2/5 labels Oct 7, 2025
Copy link

github-actions bot commented Oct 7, 2025

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

Trigger Scope
Replacing pull_request triggers with workflow_run means E2E won’t run for PRs unless the Unit Tests workflow runs in the same repository context. Validate that PRs from forks or with only docs/CI changes still trigger Unit Tests and subsequently E2E as intended.

Conditional IF
The job-level condition relies on github.event.workflow_run.conclusion == 'success'. Confirm this field exists for manual workflow_dispatch runs (it won’t) and that E2E can still be manually triggered when desired; otherwise they’ll be skipped unintentionally.

Behavior Change

Removing continue-on-error changes matrix handling; a single Python version failure now fails the workflow and blocks all E2E. Ensure this stricter gating is intended and won’t stall E2E due to transient failures in a single matrix job.

jobs:
  unit-tests:
    strategy:
      fail-fast: false
      matrix:
        python-version: ["3.9", "3.10", "3.11", "3.12", "3.13"]
    runs-on: ubuntu-latest

Copy link

github-actions bot commented Oct 7, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Guard PR-only context usage

The environment expression references github.event.pull_request but this workflow is
triggered by workflow_run, where pull_request is not present. This will evaluate to
null and can break the conditional. Guard the PR-specific fields with a check or
default them to safe values.

.github/workflows/e2e-async.yaml [11-15]

 jobs:
   async-optimization:
     if: ${{ github.event.workflow_run.conclusion == 'success' }}
-    # Dynamically determine if environment is needed only when workflow files change and contributor is external
-    environment: ${{ (github.event_name == 'workflow_dispatch' || (contains(toJSON(github.event.pull_request.files.*.filename), '.github/workflows/') && github.event.pull_request.user.login != 'misrasaurabh1' && github.event.pull_request.user.login != 'KRRT7')) && 'external-trusted-contributors' || '' }}
+    # Only evaluate PR-specific data when the event is a pull_request
+    environment: ${{ (github.event_name == 'workflow_dispatch' || (github.event_name == 'pull_request' && contains(toJSON(github.event.pull_request.files.*.filename), '.github/workflows/') && github.event.pull_request.user.login != 'misrasaurabh1' && github.event.pull_request.user.login != 'KRRT7')) && 'external-trusted-contributors' || '' }}
Suggestion importance[1-10]: 8

__

Why: Correctly identifies that github.event.pull_request is not available for workflow_run events and proposes a precise guard using github.event_name == 'pull_request', preventing null dereferences and incorrect env evaluation.

Medium
General
Prevent matrix from blocking E2E

Removing continue-on-error: true makes all matrix failures block downstream
workflow_run E2E jobs. If the intent was to proceed with E2E only when all unit
tests pass, keep as-is; otherwise, reintroduce a controlled allow-failure for
experimental versions to prevent accidental blockage.

.github/workflows/unit-tests.yaml [11-15]

 jobs:
   unit-tests:
     strategy:
       fail-fast: false
       matrix:
         python-version: ["3.9", "3.10", "3.11", "3.12", "3.13"]
+        include:
+          - python-version: "3.13"
+            allow-failure: true
     runs-on: ubuntu-latest
+    continue-on-error: ${{ matrix.allow-failure == true }}
Suggestion importance[1-10]: 6

__

Why: Sensible trade-off suggestion to allow experimental Python versions to fail without blocking E2E, using matrix-driven continue-on-error; optional but improves pipeline resilience.

Low
Restrict triggering branches

workflow_run triggers for all branches; without filtering, E2E may run on main or
unrelated branches. Add a branches filter to ensure it only runs for the intended
base (e.g., main) to avoid unintended executions.

.github/workflows/e2e-bubblesort-benchmark.yaml [3-7]

 on:
   workflow_run:
     workflows: ["Unit Tests"]
+    branches: [ main ]
     types:
       - completed
Suggestion importance[1-10]: 5

__

Why: Filtering by branch can reduce unintended runs, but adding branches under workflow_run is not supported by GitHub Actions; correct intent but the proposed YAML is invalid.

Low

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Review effort 2/5 workflow-modified This PR modifies GitHub Actions workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant