Skip to content

Remove automatic execution of test that require LLM inputs from merge… #184

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Vasilije1990 merged 3 commits into from
Nov 8, 2024
Merged

Remove automatic execution of test that require LLM inputs from merge… #184

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c62800c
Remove automatic execution of test that require LLM inputs from merge…
0xideas Nov 8, 2024
c2dfe25
Try run-checks approach
0xideas Nov 8, 2024
236a8c1
Move github actions if condition
0xideas Nov 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions .github/workflows/test_neo4j.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
name: test | neo4j

on:
workflow_dispatch:
pull_request:
branches:
- main
workflow_dispatch:
types: [labeled]

concurrency:
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
Expand All @@ -21,7 +22,7 @@ jobs:
run_neo4j_integration_test:
name: test
needs: get_docs_changes
if: needs.get_docs_changes.outputs.changes_outside_docs == 'true'
if: needs.get_docs_changes.outputs.changes_outside_docs == 'true' | ${{ github.event.label.name == 'run-checks' }}
runs-on: ubuntu-latest

defaults:
Expand Down
6 changes: 4 additions & 2 deletions .github/workflows/test_notebook.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
name: test | notebook

on:
workflow_dispatch:
pull_request:
branches:
- main
workflow_dispatch:
types: [labeled]

Comment on lines +4 to +9
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 8, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance trigger conditions to prevent unnecessary workflow runs.

While the changes align with the goal of preventing automatic test execution, the current trigger configuration could be improved to be more precise.

Apply this diff to handle both label addition and removal, and specifically filter for the 'run-checks' label:

  workflow_dispatch:
  pull_request:
    branches:
      - main
-    types: [labeled]
+    types: [labeled, unlabeled]

This ensures the workflow:

  1. Can be triggered manually when needed
  2. Runs when the 'run-checks' label is added
  3. Stops when the label is removed

Committable suggestion skipped: line range outside the PR's diff.


concurrency:
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
Expand All @@ -21,7 +23,7 @@ jobs:
run_notebook_test:
name: test
needs: get_docs_changes
if: needs.get_docs_changes.outputs.changes_outside_docs == 'true'
if: needs.get_docs_changes.outputs.changes_outside_docs == 'true' | ${{ github.event.label.name == 'run-checks' }}
runs-on: ubuntu-latest
defaults:
run:
Expand Down
6 changes: 4 additions & 2 deletions .github/workflows/test_pgvector.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
name: test | pgvector

on:
workflow_dispatch:
pull_request:
branches:
- main
workflow_dispatch:
types: [labeled]


concurrency:
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
Expand All @@ -21,7 +23,7 @@ jobs:
run_pgvector_integration_test:
name: test
needs: get_docs_changes
if: needs.get_docs_changes.outputs.changes_outside_docs == 'true'
if: needs.get_docs_changes.outputs.changes_outside_docs == 'true' | ${{ github.event.label.name == 'run-checks' }}
runs-on: ubuntu-latest
defaults:
run:
Expand Down
6 changes: 4 additions & 2 deletions .github/workflows/test_qdrant.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
name: test | qdrant

on:
workflow_dispatch:
pull_request:
branches:
- main
workflow_dispatch:
types: [labeled]
Comment on lines +4 to +8
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 8, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove pull_request trigger for main branch to align with PR objectives

The current configuration still allows automatic execution on pull requests to main, which contradicts the PR's goal of removing automatic execution of LLM-dependent tests.

Apply this diff to fix:

on:
  workflow_dispatch:
  pull_request:
-    branches:
-      - main
    types: [labeled]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
workflow_dispatch:
pull_request:
branches:
- main
workflow_dispatch:
types: [labeled]
workflow_dispatch:
pull_request:
types: [labeled]



concurrency:
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
Expand All @@ -21,7 +23,7 @@ jobs:
run_qdrant_integration_test:
name: test
needs: get_docs_changes
if: needs.get_docs_changes.outputs.changes_outside_docs == 'true'
if: needs.get_docs_changes.outputs.changes_outside_docs == 'true' | ${{ github.event.label.name == 'run-checks' }}
runs-on: ubuntu-latest

defaults:
Expand Down
6 changes: 4 additions & 2 deletions .github/workflows/test_weaviate.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
name: test | weaviate

on:
workflow_dispatch:
pull_request:
branches:
- main
workflow_dispatch:
types: [labeled]

Comment on lines +4 to +9
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 8, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Workflow still triggers automatically on PRs to main branch

The current configuration contradicts the PR objective of removing automatic test execution on merge requests to main. While the label condition was added, the workflow will still trigger automatically when PRs are labeled.

To align with the PR objective, modify the trigger configuration:

on:
  workflow_dispatch:
- pull_request:
-   branches:
-     - main
-   types: [labeled]

This ensures the workflow only runs when manually triggered via workflow_dispatch.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
workflow_dispatch:
pull_request:
branches:
- main
workflow_dispatch:
types: [labeled]
workflow_dispatch:


concurrency:
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
Expand All @@ -21,7 +23,7 @@ jobs:
run_weaviate_integration_test:
name: test
needs: get_docs_changes
if: needs.get_docs_changes.outputs.changes_outside_docs == 'true'
if: needs.get_docs_changes.outputs.changes_outside_docs == 'true' | ${{ github.event.label.name == 'run-checks' }}
runs-on: ubuntu-latest

defaults:
Expand Down
Loading