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 2 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: 4 additions & 1 deletion .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 @@ -14,6 +15,8 @@ env:
RUNTIME__LOG_LEVEL: ERROR

jobs:
if: ${{ github.event.label.name == 'run-checks' }}

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

Fix invalid YAML structure.

The current placement of the if condition is incorrect and will cause the workflow to fail. The condition should be moved into each job that needs to be controlled by the label.

Apply this change to fix the YAML structure:

jobs:
- if: ${{ github.event.label.name == 'run-checks' }}

  get_docs_changes:
    name: docs changes
    uses: ./.github/workflows/get_docs_changes.yml
+   if: ${{ github.event.label.name == 'run-checks' }}

  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
📝 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
if: ${{ github.event.label.name == 'run-checks' }}
get_docs_changes:
name: docs changes
uses: ./.github/workflows/get_docs_changes.yml
if: ${{ github.event.label.name == 'run-checks' }}
run_neo4j_integration_test:
name: test
needs: get_docs_changes
if: |
needs.get_docs_changes.outputs.changes_outside_docs == 'true' &&
github.event.label.name == 'run-checks'
runs-on: ubuntu-latest
🧰 Tools
🪛 actionlint

18-18: "steps" section is missing in job "if"

(syntax-check)

18-18: "runs-on" section is missing in job "if"

(syntax-check)

18-18: "if" job is scalar node but mapping node is expected

(syntax-check)

get_docs_changes:
name: docs changes
uses: ./.github/workflows/get_docs_changes.yml
Expand Down
6 changes: 5 additions & 1 deletion .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 @@ -14,6 +16,8 @@ env:
RUNTIME__LOG_LEVEL: ERROR

jobs:
if: ${{ github.event.label.name == 'run-checks' }}

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

Fix YAML structure for conditional execution.

The current job-level condition is incorrectly structured and will cause syntax errors.

Apply this diff to fix the YAML structure:

jobs:
-  if: ${{ github.event.label.name == 'run-checks' }}
-
  get_docs_changes:
    name: docs changes
+    if: github.event.label.name == 'run-checks' || github.event_name == 'workflow_dispatch'
    uses: ./.github/workflows/get_docs_changes.yml

  run_notebook_test:
    name: test
    needs: get_docs_changes
-    if: needs.get_docs_changes.outputs.changes_outside_docs == 'true'
+    if: (github.event.label.name == 'run-checks' || github.event_name == 'workflow_dispatch') && needs.get_docs_changes.outputs.changes_outside_docs == 'true'

This change:

  1. Fixes the YAML syntax error
  2. Applies the label condition correctly to each job
  3. Preserves the existing docs changes check
  4. Allows execution via manual 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
if: ${{ github.event.label.name == 'run-checks' }}
jobs:
get_docs_changes:
name: docs changes
if: github.event.label.name == 'run-checks' || github.event_name == 'workflow_dispatch'
uses: ./.github/workflows/get_docs_changes.yml
run_notebook_test:
name: test
needs: get_docs_changes
if: (github.event.label.name == 'run-checks' || github.event_name == 'workflow_dispatch') && needs.get_docs_changes.outputs.changes_outside_docs == 'true'
🧰 Tools
🪛 actionlint

19-19: "steps" section is missing in job "if"

(syntax-check)

19-19: "runs-on" section is missing in job "if"

(syntax-check)

19-19: "if" job is scalar node but mapping node is expected

(syntax-check)

get_docs_changes:
name: docs changes
uses: ./.github/workflows/get_docs_changes.yml
Expand Down
6 changes: 5 additions & 1 deletion .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 @@ -14,6 +16,8 @@ env:
RUNTIME__LOG_LEVEL: ERROR

jobs:
if: ${{ github.event.label.name == 'run-checks' }}

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

Fix critical YAML syntax error in job conditions.

The if condition is incorrectly placed at the jobs level, causing syntax errors. It should be moved into each individual job.

jobs:
-  if: ${{ github.event.label.name == 'run-checks' }}

  get_docs_changes:
    name: docs changes
    uses: ./.github/workflows/get_docs_changes.yml
+   if: ${{ github.event.label.name == 'run-checks' }}

  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'
📝 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
if: ${{ github.event.label.name == 'run-checks' }}
jobs:
get_docs_changes:
name: docs changes
uses: ./.github/workflows/get_docs_changes.yml
if: ${{ github.event.label.name == 'run-checks' }}
run_pgvector_integration_test:
name: test
needs: get_docs_changes
if: |
needs.get_docs_changes.outputs.changes_outside_docs == 'true' &&
github.event.label.name == 'run-checks'
🧰 Tools
🪛 actionlint

19-19: "steps" section is missing in job "if"

(syntax-check)

19-19: "runs-on" section is missing in job "if"

(syntax-check)

19-19: "if" job is scalar node but mapping node is expected

(syntax-check)

get_docs_changes:
name: docs changes
uses: ./.github/workflows/get_docs_changes.yml
Expand Down
6 changes: 5 additions & 1 deletion .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 @@ -14,6 +16,8 @@ env:
RUNTIME__LOG_LEVEL: ERROR

jobs:
if: ${{ github.event.label.name == 'run-checks' }}

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

Fix incorrect YAML structure for job conditions

The conditional check is incorrectly placed as a separate job. It should be moved to the individual jobs that need this condition.

Apply this diff to fix:

jobs:
-  if: ${{ github.event.label.name == 'run-checks' }}

  get_docs_changes:
    name: docs changes
    uses: ./.github/workflows/get_docs_changes.yml

  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
📝 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
if: ${{ github.event.label.name == 'run-checks' }}
jobs:
get_docs_changes:
name: docs changes
uses: ./.github/workflows/get_docs_changes.yml
run_qdrant_integration_test:
name: test
needs: get_docs_changes
if: |
needs.get_docs_changes.outputs.changes_outside_docs == 'true' &&
github.event.label.name == 'run-checks'
runs-on: ubuntu-latest
🧰 Tools
🪛 actionlint

19-19: "steps" section is missing in job "if"

(syntax-check)

19-19: "runs-on" section is missing in job "if"

(syntax-check)

19-19: "if" job is scalar node but mapping node is expected

(syntax-check)

get_docs_changes:
name: docs changes
uses: ./.github/workflows/get_docs_changes.yml
Expand Down
6 changes: 5 additions & 1 deletion .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 @@ -14,6 +16,8 @@ env:
RUNTIME__LOG_LEVEL: ERROR

jobs:
if: ${{ github.event.label.name == 'run-checks' }}

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

Fix YAML structure: Incorrect placement of if condition

The label condition is incorrectly placed as a separate job. This will cause the workflow to fail.

Move the condition to each job that needs it:

jobs:
- if: ${{ github.event.label.name == 'run-checks' }}

  get_docs_changes:
    name: docs changes
+   if: ${{ github.event.label.name == 'run-checks' }}
    uses: ./.github/workflows/get_docs_changes.yml

  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'
📝 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
if: ${{ github.event.label.name == 'run-checks' }}
get_docs_changes:
name: docs changes
if: ${{ github.event.label.name == 'run-checks' }}
uses: ./.github/workflows/get_docs_changes.yml
run_weaviate_integration_test:
name: test
needs: get_docs_changes
if: |
needs.get_docs_changes.outputs.changes_outside_docs == 'true' &&
github.event.label.name == 'run-checks'
🧰 Tools
🪛 actionlint

19-19: "steps" section is missing in job "if"

(syntax-check)

19-19: "runs-on" section is missing in job "if"

(syntax-check)

19-19: "if" job is scalar node but mapping node is expected

(syntax-check)

get_docs_changes:
name: docs changes
uses: ./.github/workflows/get_docs_changes.yml
Expand Down
Loading