ci: remove unnecessary tests and glific mocks#219
Open
rvignesh89 wants to merge 25 commits intomainfrom
Open
Conversation
📝 WalkthroughWalkthroughAdds CI workflows for standard and slow E2E runs with ngrok tunneling, provisions backend/frontend and PostgreSQL, updates Cypress spec to remove many network mocks and rely on real interactions, pins Node.js via Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor GitHubActions as "GitHub Actions Runner"
participant NGROK as "ngrok"
participant Postgres as "PostgreSQL (service)"
participant Backend as "Elixir Backend"
participant Frontend as "Frontend (Yarn)"
participant Cypress as "Cypress Runner"
participant Artifacts as "Artifact Store"
GitHubActions->>Postgres: start DB service (healthcheck pg_isready)
GitHubActions->>NGROK: download & start tunnel (port 4000)
NGROK-->>GitHubActions: return public_url
GitHubActions->>Backend: clone repo, write certs, env, start (mix phx.server)
GitHubActions->>Frontend: clone repo, setup env, start (yarn dev)
GitHubActions->>Backend: wait-on https-get://glific.test:4001
GitHubActions->>Cypress: run tests (with GLIFIC_API_HOST_OVERRIDE)
Cypress->>Backend: API requests (real)
Cypress->>Frontend: UI interactions
GitHubActions->>Artifacts: upload phoenix-server.log, ngrok.log (always)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 17660795 | Triggered | Generic Password | d9dc9d1 | .github/workflows/e2e-slow.yml | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
rvignesh89
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
cypress/e2e/filesearch/Filesearch.spec.ts (3)
36-36: Replace staticcy.wait(500)with an assertion.Static waits are flaky and slow. Wait for a specific condition instead, such as the form becoming interactive.
Proposed fix
cy.get('[data-testid="headingButton"]').click(); - cy.wait(500); + cy.get('[data-testid="createAssistantContainer"]').should('be.visible'); cy.get('input[name="name"]').type(assistantName);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cypress/e2e/filesearch/Filesearch.spec.ts` at line 36, Replace the static cy.wait(500) in Filesearch.spec.ts with a deterministic assertion that the UI is ready; locate the cy.wait(500) call and change it to an assertion that the search form or input is present and interactive (for example assert the search input or submit button exists/visible/enabled via cy.get('<your-form-or-input-selector>').should('be.visible').and('be.enabled') or cy.get('<form-selector>').should('exist') so the test waits for a concrete condition instead of a fixed timeout.
116-116: Replace staticcy.wait(500)with an assertion.Same as line 36—prefer waiting for a UI condition.
Proposed fix
cy.get('input[placeholder="Search"]').type('SomeSearchTerm'); - cy.wait(500); + cy.get('[data-testid="listItem"]').should('exist'); // or appropriate assertion cy.get('[data-testid="CloseIcon"]').click();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cypress/e2e/filesearch/Filesearch.spec.ts` at line 116, Replace the static cy.wait(500) in Filesearch.spec.ts with a deterministic assertion: instead of sleeping, wait for a specific UI condition or network alias that indicates the state change (for example, assert that the file list element is visible, contains expected text, or that a stubbed XHR/route alias has completed); locate the second occurrence at the cy.wait(500) around line ~116 (same pattern as the earlier cy.wait at ~36) and change it to an assertion on the relevant DOM element or use cy.wait('@alias') so the test proceeds only when the UI or network is ready.
1-9: Test suite has implicit inter-test dependencies.Tests rely on state from previous tests (e.g.,
assistantNamecreated in one test is searched/updated/deleted in subsequent tests). If tests run in isolation or out of order, they will fail. While this is intentional for an end-to-end workflow, consider:
- Adding a comment at the top explaining the sequential dependency requirement
- Ensuring Cypress is configured to run specs in order (default behavior)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cypress/e2e/filesearch/Filesearch.spec.ts` around lines 1 - 9, This test suite relies on state carried between tests (assistantName / updatedName) and must be run sequentially; add a clear top-of-file comment above describe('File search', ...) stating that tests are inter-dependent and must run in order, and optionally reference Cypress run-order configuration, and also consider documenting that beforeEach (cy.login, cy.visit) does not reset assistant state—if you prefer isolation later, convert dependent tests to create and clean up their own assistant within their individual it() blocks or add a dedicated setup/teardown helper to create/delete the assistant rather than relying on a previously created assistant.README.md (1)
35-37: Add language identifier to fenced code block.The code block should specify a language for proper syntax highlighting.
Proposed fix
-``` +```bash yarn cypress run --config excludeSpecPattern=cypress/e2e/filesearch/Filesearch.spec.ts</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@README.mdaround lines 35 - 37, The fenced code block that shows the Cypress
command is missing a language identifier; update the block around the command
yarn cypress run --config excludeSpecPattern=cypress/e2e/filesearch/Filesearch.spec.tsto use a language
tag (e.g., add ```bash) so syntax highlighting works in README.md.</details> </blockquote></details> <details> <summary>.tool-versions (1)</summary><blockquote> `1-1`: **CI workflows do not consume `.tool-versions` for Node.js version.** The `setup-node@v6` action in both `e2e.yml` (line 48) and `e2e-slow.yml` (line 40) lacks a `node-version-file` parameter, so CI will use the runner's default Node.js version rather than `22.21.1`. To ensure consistency between local and CI environments: <details> <summary>Proposed fix for workflow files</summary> ```diff - name: Use latest Node.js uses: actions/setup-node@v6 + with: + node-version-file: '.tool-versions' ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In @.tool-versions at line 1, CI is not reading the .tool-versions Node.js value because the setup-node@v6 steps in e2e.yml and e2e-slow.yml omit the node-version-file parameter; update the GitHub Actions jobs that call actions/setup-node@v6 (search for the setup-node usage in e2e.yml and e2e-slow.yml) to add node-version-file: ".tool-versions" (or pass node-version: 22.21.1 if you prefer an explicit value) so the runner uses the repository's Node.js version. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/e2e-slow.yml:
- Around line 6-9: The workflow's on: pull_request types list includes an
invalid activity "push" which fails validation; edit the on: block by removing
"push" from the pull_request.types array (reference: the pull_request.types
entry in the .github/workflows/e2e-slow.yml diff) and, if you intended to run
this workflow on push events as well, add a separate top-level "push:" trigger
instead of placing "push" inside pull_request.types.In @.github/workflows/e2e.yml:
- Line 102: The workflow is cloning a feature branch literal ("git clone
--branch rvignesh/seed-dev-kaapi ...") which can disappear after the PR; update
that command to clone a stable branch (e.g., replace the branch name with
"main") or make it dynamic by using the GitHub workflow variable (for example
use the repository default branch or "${{ github.ref }}" / "${{ github.head_ref
}}" as appropriate) so the CI does not depend on a temporary feature branch;
locate the git clone command string ("git clone --branch
rvignesh/seed-dev-kaapi") and change the branch argument accordingly.- Around line 71-91: The workflow exports GLIFIC_API_HOST_OVERRIDE but Cypress
uses hardcoded URLs (baseUrl and backendUrl), so update Cypress config (e.g.,
cypress.config.js or cypress.json) to read process.env.GLIFIC_API_HOST_OVERRIDE
and derive baseUrl and backendUrl when that env var is set (use
GLIFIC_API_HOST_OVERRIDE as the host/hostname and append the existing
ports/paths), ensuring symbols like baseUrl and backendUrl in the Cypress
configuration are assigned from process.env.GLIFIC_API_HOST_OVERRIDE if present;
alternatively remove the GLIFIC_API_HOST_OVERRIDE export from the workflow if
ngrok tunneling is not needed.In
@cypress/e2e/filesearch/Filesearch.spec.ts:
- Around line 69-96: The current recursive polling function checkStatusOrRetry
(with maxTries and tryCount) is fragile and the final return doesn't make
Cypress wait; replace this recursive pattern with Cypress' retryable commands or
the cypress-recurse utility: remove checkStatusOrRetry recursion and instead use
cy.recurse (or a cy.get(...).should(...) loop) to repeatedly read
'[data-testid="assistantStatus"] span' until its trimmed text equals 'Ready' (or
until maxTries/retry timeout is reached), performing cy.reload() between
attempts if needed and using configurable interval/timeout instead of a fixed
cy.wait(5000); ensure the test returns the Cypress chain (the cy.recurse /
cy.get(...).should(...) promise) so Cypress properly waits for the status to
become Ready and eliminate tryCount/return checkStatusOrRetry recursion.
Nitpick comments:
In @.tool-versions:
- Line 1: CI is not reading the .tool-versions Node.js value because the
setup-node@v6 steps in e2e.yml and e2e-slow.yml omit the node-version-file
parameter; update the GitHub Actions jobs that call actions/setup-node@v6
(search for the setup-node usage in e2e.yml and e2e-slow.yml) to add
node-version-file: ".tool-versions" (or pass node-version: 22.21.1 if you prefer
an explicit value) so the runner uses the repository's Node.js version.In
@cypress/e2e/filesearch/Filesearch.spec.ts:
- Line 36: Replace the static cy.wait(500) in Filesearch.spec.ts with a
deterministic assertion that the UI is ready; locate the cy.wait(500) call and
change it to an assertion that the search form or input is present and
interactive (for example assert the search input or submit button
exists/visible/enabled via
cy.get('').should('be.visible').and('be.enabled')
or cy.get('').should('exist') so the test waits for a concrete
condition instead of a fixed timeout.- Line 116: Replace the static cy.wait(500) in Filesearch.spec.ts with a
deterministic assertion: instead of sleeping, wait for a specific UI condition
or network alias that indicates the state change (for example, assert that the
file list element is visible, contains expected text, or that a stubbed
XHR/route alias has completed); locate the second occurrence at the cy.wait(500)
around line ~116 (same pattern as the earlier cy.wait at ~36) and change it to
an assertion on the relevant DOM element or use cy.wait('@alias') so the test
proceeds only when the UI or network is ready.- Around line 1-9: This test suite relies on state carried between tests
(assistantName / updatedName) and must be run sequentially; add a clear
top-of-file comment above describe('File search', ...) stating that tests are
inter-dependent and must run in order, and optionally reference Cypress
run-order configuration, and also consider documenting that beforeEach
(cy.login, cy.visit) does not reset assistant state—if you prefer isolation
later, convert dependent tests to create and clean up their own assistant within
their individual it() blocks or add a dedicated setup/teardown helper to
create/delete the assistant rather than relying on a previously created
assistant.In
@README.md:
- Around line 35-37: The fenced code block that shows the Cypress command is
missing a language identifier; update the block around the commandyarn cypress run --config excludeSpecPattern=cypress/e2e/filesearch/Filesearch.spec.tsto
use a language tag (e.g., add ```bash) so syntax highlighting works in
README.md.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `44f4f897-5e38-4747-b954-a4a7c0475938` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 52e21a27d01225a600d3214ed5fed4509fc850b7 and eb403b667ac1c8201fcead89c3f575ec2bcee8ed. </details> <details> <summary>📒 Files selected for processing (5)</summary> * `.github/workflows/e2e-slow.yml` * `.github/workflows/e2e.yml` * `.tool-versions` * `README.md` * `cypress/e2e/filesearch/Filesearch.spec.ts` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Comment on lines
+6
to
+9
| on: | ||
| pull_request: | ||
| types: [opened, synchronize, reopened, labeled, unlabeled, push] | ||
| branches: [main] |
There was a problem hiding this comment.
Invalid activity type push for pull_request event.
The push activity type is not valid for the pull_request webhook event and will cause the workflow to fail validation.
Proposed fix
on:
pull_request:
- types: [opened, synchronize, reopened, labeled, unlabeled, push]
+ types: [opened, synchronize, reopened, labeled, unlabeled]
branches: [main]📝 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
| on: | |
| pull_request: | |
| types: [opened, synchronize, reopened, labeled, unlabeled, push] | |
| branches: [main] | |
| on: | |
| pull_request: | |
| types: [opened, synchronize, reopened, labeled, unlabeled] | |
| branches: [main] |
🧰 Tools
🪛 actionlint (1.7.11)
[error] 8-8: invalid activity type "push" for "pull_request" Webhook event. available types are "assigned", "auto_merge_disabled", "auto_merge_enabled", "closed", "converted_to_draft", "demilestoned", "dequeued", "edited", "enqueued", "labeled", "locked", "milestoned", "opened", "ready_for_review", "reopened", "review_request_removed", "review_requested", "synchronize", "unassigned", "unlabeled", "unlocked"
(events)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/e2e-slow.yml around lines 6 - 9, The workflow's on:
pull_request types list includes an invalid activity "push" which fails
validation; edit the on: block by removing "push" from the pull_request.types
array (reference: the pull_request.types entry in the
.github/workflows/e2e-slow.yml diff) and, if you intended to run this workflow
on push events as well, add a separate top-level "push:" trigger instead of
placing "push" inside pull_request.types.
There was a problem hiding this comment.
@rvignesh89, sounds good! Just a reminder that the fix is simply removing push from the pull_request.types array:
on:
pull_request:
types: [opened, synchronize, reopened, labeled, unlabeled]
branches: [main]If you also intended to trigger this workflow on direct pushes to main, that would need a separate top-level push: trigger block. Let me know if you need any help with that!
🧠 Learnings used
Learnt from: rvignesh89
Repo: glific/cypress-testing PR: 219
File: .github/workflows/e2e.yml:71-91
Timestamp: 2026-04-01T02:44:21.649Z
Learning: When reviewing e2e GitHub Actions workflows in this repo, don’t assume `GLIFIC_API_HOST_OVERRIDE` is unused just because Cypress tests/config contain hardcoded URLs. This variable is intended for the Glific backend commands executed in the workflow (e.g., `mix setup` and `mix phx.server`), not for Cypress itself. Only flag/consider changes to Cypress URLs if there’s evidence the backend/API host resolution behavior is affected.
rvignesh89
commented
Apr 1, 2026
.github/workflows/e2e-slow.yml
Outdated
| mkdir project | ||
| cd project | ||
| echo clone glific repo | ||
| git clone --branch rvignesh/seed-dev-kaapi https://github.com/glific/glific.git |
Author
There was a problem hiding this comment.
This branch will be changed once glific PR is merged to master
akanshaaa19
approved these changes
Apr 8, 2026
There was a problem hiding this comment.
♻️ Duplicate comments (3)
.github/workflows/e2e.yml (1)
102-102:⚠️ Potential issue | 🟠 MajorReplace temporary branch clone with a stable ref.
Line 102 depends on
rvignesh/seed-dev-kaapi; CI will become fragile once that branch is deleted. Switch to a stable branch/ref before merge.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/e2e.yml at line 102, The workflow currently clones a temporary branch name (rvignesh/seed-dev-kaapi) which will break CI if that branch is removed; update the git clone invocation in .github/workflows/e2e.yml to use a stable ref instead (for example replace the branch argument with a maintained branch like main or a specific release tag or a pinned commit SHA), i.e., change the clone command that references "rvignesh/seed-dev-kaapi" so it points to a stable branch/tag/SHA before merging to ensure CI remains reliable..github/workflows/e2e-slow.yml (2)
89-89:⚠️ Potential issue | 🟠 MajorAvoid pinning CI to a temporary feature branch.
Line 89 clones
rvignesh/seed-dev-kaapi; this can break CI once that branch is removed. Prefer a stable branch (for examplemain) or a workflow input/ref.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/e2e-slow.yml at line 89, The workflow is pinning CI to a temporary feature branch via the git clone command that uses the branch name "rvignesh/seed-dev-kaapi"; change that git clone invocation to use a stable ref (e.g., "main") or replace the hardcoded branch with a workflow input/variable (e.g., github.ref or a workflow input like inputs.branch) so the CI doesn't break when the feature branch is deleted.
8-8:⚠️ Potential issue | 🔴 CriticalRemove invalid
pushactivity frompull_request.types(workflow validation blocker).Line 8 includes
pushunderpull_request.types, which is invalid and can fail workflow validation.Proposed fix
on: pull_request: - types: [opened, synchronize, reopened, labeled, unlabeled, push] + types: [opened, synchronize, reopened, labeled, unlabeled] branches: [main]#!/bin/bash # Verify invalid activity type in pull_request.types rg -nP '^\s*types:\s*\[.*\bpush\b.*\]' .github/workflows/e2e-slow.yml🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/e2e-slow.yml at line 8, Remove the invalid "push" activity from the pull_request.types array so the workflow passes validation: locate the pull_request trigger block and its types key (the line containing types: [opened, synchronize, reopened, labeled, unlabeled, push]) and delete only the "push" entry (and any trailing comma if needed) so the list contains valid pull_request event types (e.g., opened, synchronize, reopened, labeled, unlabeled).
🧹 Nitpick comments (2)
.github/workflows/e2e.yml (1)
169-177: Replace hardcoded workspace paths with${{ github.workspace }}.Lines 169, 176, 202, and 203 use fixed runner paths (
/home/runner/work/cypress-testing/cypress-testing/...), which are brittle for forks and repository path changes. Using the${{ github.workspace }}variable ensures portability across different workflow runs and fork configurations.Proposed refactor
- cd /home/runner/work/cypress-testing/cypress-testing/project/glific-frontend + cd "${{ github.workspace }}/project/glific-frontend" yarn dev & ... - cd /home/runner/work/cypress-testing/cypress-testing/project/glific + cd "${{ github.workspace }}/project/glific" ENABLE_DB_SSL=false OPEN_AI_KEY=${{secrets.OPENAI_KEY}} mix phx.server > phoenix-server.log 2>&1 & ... - /home/runner/work/cypress-testing/cypress-testing/project/glific/phoenix-server.log + ${{ github.workspace }}/project/glific/phoenix-server.log - /home/runner/work/cypress-testing/cypress-testing/ngrok.log + ${{ github.workspace }}/ngrok.log🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/e2e.yml around lines 169 - 177, Replace the hardcoded runner paths used in the workflow steps that change directories and start services with the portable GitHub Actions workspace variable: update the commands that do "cd /home/runner/work/cypress-testing/cypress-testing/project/glific-frontend" (followed by "yarn dev &") and "cd /home/runner/work/cypress-testing/cypress-testing/project/glific" (followed by "ENABLE_DB_SSL=false OPEN_AI_KEY=${{secrets.OPENAI_KEY}} mix phx.server > phoenix-server.log 2>&1 &") to use "${{ github.workspace }}/project/..." instead of the fixed absolute path so the steps (triggering yarn dev and the "run glific" mix phx.server) work for forks and different repo paths..github/workflows/e2e-slow.yml (1)
154-160: Use${{ github.workspace }}instead of hardcoded runner paths.Lines 154 and 159 hardcode
/home/runner/work/cypress-testing/cypress-testing/; this breaks portability across forks and workspace configurations.Proposed refactor
- cd /home/runner/work/cypress-testing/cypress-testing/project/glific-frontend + cd "${{ github.workspace }}/project/glific-frontend" yarn dev & ... - cd /home/runner/work/cypress-testing/cypress-testing/project/glific + cd "${{ github.workspace }}/project/glific" ENABLE_DB_SSL=false OPEN_AI_KEY=${{secrets.OPENAI_KEY}} mix phx.server > phoenix-server.log 2>&1 &🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/e2e-slow.yml around lines 154 - 160, The workflow hardcodes the runner path in the "run glific" and earlier step; replace occurrences of "/home/runner/work/cypress-testing/cypress-testing" with the workflow variable ${{ github.workspace }} so the cd commands use "${{ github.workspace }}/project/glific-frontend" and "${{ github.workspace }}/project/glific" (respectively) to maintain portability across forks/workspaces; update any related steps that reference the same hardcoded path to use ${{ github.workspace }} as well.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/e2e-slow.yml:
- Line 89: The workflow is pinning CI to a temporary feature branch via the git
clone command that uses the branch name "rvignesh/seed-dev-kaapi"; change that
git clone invocation to use a stable ref (e.g., "main") or replace the hardcoded
branch with a workflow input/variable (e.g., github.ref or a workflow input like
inputs.branch) so the CI doesn't break when the feature branch is deleted.
- Line 8: Remove the invalid "push" activity from the pull_request.types array
so the workflow passes validation: locate the pull_request trigger block and its
types key (the line containing types: [opened, synchronize, reopened, labeled,
unlabeled, push]) and delete only the "push" entry (and any trailing comma if
needed) so the list contains valid pull_request event types (e.g., opened,
synchronize, reopened, labeled, unlabeled).
In @.github/workflows/e2e.yml:
- Line 102: The workflow currently clones a temporary branch name
(rvignesh/seed-dev-kaapi) which will break CI if that branch is removed; update
the git clone invocation in .github/workflows/e2e.yml to use a stable ref
instead (for example replace the branch argument with a maintained branch like
main or a specific release tag or a pinned commit SHA), i.e., change the clone
command that references "rvignesh/seed-dev-kaapi" so it points to a stable
branch/tag/SHA before merging to ensure CI remains reliable.
---
Nitpick comments:
In @.github/workflows/e2e-slow.yml:
- Around line 154-160: The workflow hardcodes the runner path in the "run
glific" and earlier step; replace occurrences of
"/home/runner/work/cypress-testing/cypress-testing" with the workflow variable
${{ github.workspace }} so the cd commands use "${{ github.workspace
}}/project/glific-frontend" and "${{ github.workspace }}/project/glific"
(respectively) to maintain portability across forks/workspaces; update any
related steps that reference the same hardcoded path to use ${{ github.workspace
}} as well.
In @.github/workflows/e2e.yml:
- Around line 169-177: Replace the hardcoded runner paths used in the workflow
steps that change directories and start services with the portable GitHub
Actions workspace variable: update the commands that do "cd
/home/runner/work/cypress-testing/cypress-testing/project/glific-frontend"
(followed by "yarn dev &") and "cd
/home/runner/work/cypress-testing/cypress-testing/project/glific" (followed by
"ENABLE_DB_SSL=false OPEN_AI_KEY=${{secrets.OPENAI_KEY}} mix phx.server >
phoenix-server.log 2>&1 &") to use "${{ github.workspace }}/project/..." instead
of the fixed absolute path so the steps (triggering yarn dev and the "run
glific" mix phx.server) work for forks and different repo paths.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a946a99e-5170-4239-a01b-d9b6be12a18a
📒 Files selected for processing (4)
.github/workflows/e2e-slow.yml.github/workflows/e2e.ymlcypress.config.ts.examplecypress/e2e/filesearch/Filesearch.spec.ts
💤 Files with no reviewable changes (1)
- cypress.config.ts.example
🚧 Files skipped from review as they are similar to previous changes (1)
- cypress/e2e/filesearch/Filesearch.spec.ts
rvignesh89
added a commit
to glific/glific
that referenced
this pull request
Apr 9, 2026
This PR upserts the Kaapi credentials in dev seeds which is used for testing. It fetches the seeds from an env var `KAAPI_API_KEY`. If they env var is missing a warning log line is written and the process continues. This is necessary so that when running cypress-tests we can configure kaapi credentials in CI and test the E2E flows. glific/cypress-testing#219
rvignesh89
force-pushed
the
rvignesh/unmock-filesearch
branch
from
78a8145 to
20a7d18
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/e2e-slow.yml (1)
39-40: Pin Node.js version for reproducible builds.The workflow uses
actions/setup-node@v6without specifying anode-version, relying on the runner's pre-installed version. This can lead to non-deterministic builds if the default Node.js version onubuntu-latestchanges.📌 Suggested fix to pin Node.js version
- name: Use latest Node.js uses: actions/setup-node@v6 + with: + node-version: '20'Alternatively, align with the version used in your other workflows or specified in
.nvmrcif present.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/e2e-slow.yml around lines 39 - 40, The workflow step "Use latest Node.js" currently uses actions/setup-node@v6 without a pinned Node version, causing non-reproducible builds; update that step (actions/setup-node@v6 / step name "Use latest Node.js") to add a pinned node-version under with: (e.g., node-version: '18' or read from .nvmrc) or replace with a specific setup action that includes the desired version so the workflow always uses a fixed Node.js release.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/e2e-slow.yml:
- Around line 39-40: The workflow step "Use latest Node.js" currently uses
actions/setup-node@v6 without a pinned Node version, causing non-reproducible
builds; update that step (actions/setup-node@v6 / step name "Use latest
Node.js") to add a pinned node-version under with: (e.g., node-version: '18' or
read from .nvmrc) or replace with a specific setup action that includes the
desired version so the workflow always uses a fixed Node.js release.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 963d6fea-9ab5-4acf-a49f-c57fb6aaeb3c
📒 Files selected for processing (1)
.github/workflows/e2e-slow.yml
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
.github/workflows/e2e.yml (1)
51-51:⚠️ Potential issue | 🟠 MajorPin third-party Actions to commit SHAs.
erlef/setup-beam@v1andiFaxity/wait-on-action@v1.2.1are mutable refs. Use full commit SHAs to reduce supply-chain risk.#!/bin/bash set -euo pipefail rg -n 'uses:\s*(erlef/setup-beam|iFaxity/wait-on-action)@' .github/workflows/e2e.ymlAlso applies to: 180-180
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/e2e.yml at line 51, The workflow uses mutable action refs erlef/setup-beam@v1 and iFaxity/wait-on-action@v1.2.1 which should be pinned to immutable commit SHAs; update each uses: reference to the full commit SHA for erlef/setup-beam and for iFaxity/wait-on-action (replace the `@v1` / `@v1.2.1` suffix with @<commit-sha>) so the workflow always runs a fixed, auditable version, and ensure you replace both occurrences (including the other occurrence noted by the reviewer)..github/workflows/e2e-slow.yml (2)
43-43:⚠️ Potential issue | 🟠 MajorPin third-party Actions by commit SHA.
erlef/setup-beam@v1andiFaxity/wait-on-action@v1.2.1should be pinned to immutable SHAs.#!/bin/bash set -euo pipefail rg -n 'uses:\s*(erlef/setup-beam|iFaxity/wait-on-action)@' .github/workflows/e2e-slow.ymlAlso applies to: 163-163
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/e2e-slow.yml at line 43, Replace the floating action tags with immutable commit SHAs for both erlef/setup-beam@v1 and iFaxity/wait-on-action@v1.2.1 in the workflow: locate the uses entries referencing "erlef/setup-beam@v1" and "iFaxity/wait-on-action@v1.2.1" and swap the tag names for the corresponding full commit SHA from each action's repository (use the action repo's git refs to obtain the exact 40-character SHA), then update the workflow to use e.g. erlef/setup-beam@<sha> and iFaxity/wait-on-action@<sha> so the actions are pinned immutably.
8-8:⚠️ Potential issue | 🔴 CriticalRemove invalid
pushactivity frompull_request.types.Line [8] uses an unsupported activity type for
pull_request, which can fail workflow validation.Suggested fix
- types: [opened, synchronize, reopened, labeled, unlabeled, push] + types: [opened, synchronize, reopened, labeled, unlabeled]#!/bin/bash set -euo pipefail rg -n 'types:\s*\[[^]]*\bpush\b[^]]*\]' .github/workflows/e2e-slow.yml🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/e2e-slow.yml at line 8, The pull_request event's types array incorrectly contains the unsupported activity "push" (see the entry "types: [opened, synchronize, reopened, labeled, unlabeled, push]"); remove "push" from the pull_request.types list so it only contains valid activities (e.g., opened, synchronize, reopened, labeled, unlabeled) to pass workflow validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/e2e-slow.yml:
- Around line 48-53: Add integrity verification for the "Download ngrok" step:
fetch the official checksum or signature alongside the ngrok archive, verify the
downloaded file (e.g., compare sha256 checksum or verify GPG signature) before
making it executable and moving it to /usr/local/bin; if verification fails,
exit the job with a clear error. Ensure verification happens after downloading
ngrok.tgz (the file produced by wget -q -O ngrok.tgz https://... ) and before
chmod +x ngrok and sudo mv ngrok /usr/local/bin/, and fail fast on mismatch so a
tampered binary is never executed or installed.
- Around line 62-80: The step that computes API_URL from NGROK_API_URL may leave
API_URL empty but still exports GLIFIC_API_HOST_OVERRIDE; update the step to
validate API_URL (the variable produced by jq from NGROK_API_URL) before
appending to $GITHUB_ENV: if API_URL is empty or null, fail the job with a clear
error and non-zero exit (or optionally set a safe default) instead of writing an
empty GLIFIC_API_HOST_OVERRIDE, and log the NGROK_API_URL/curl output for
debugging; make this change in the block that sets API_URL and writes
GLIFIC_API_HOST_OVERRIDE so downstream backend setup/server steps don’t receive
an empty value.
In @.github/workflows/e2e.yml:
- Around line 71-91: The step that reads ngrok tunnels (NGROK_API_URL) may leave
API_URL empty and still write GLIFIC_API_HOST_OVERRIDE, causing downstream
nondeterministic failures; after the existing retries and selection logic for
API_URL, add a guard that checks if API_URL is empty and if so echo a clear
error (including the ngrok API output for debugging) and exit 1 so the workflow
fails fast, and only append "GLIFIC_API_HOST_OVERRIDE=${API_URL}" to $GITHUB_ENV
when API_URL is non-empty; reference the variables/API logic named
NGROK_API_URL, API_URL and the env key GLIFIC_API_HOST_OVERRIDE to locate where
to add this check.
- Around line 56-62: Add SHA256 checksum verification for the "Download ngrok"
step: fetch the ngrok tarball as before but also fetch or read the expected
SHA256 hash from a repository secret (e.g., NGROK_SHA256) and compute the
downloaded file's checksum with sha256sum (or shasum -a 256), then compare and
fail the workflow if they differ before making the file executable or running
sudo mv; ensure the verification happens prior to chmod +x ngrok and sudo mv
ngrok /usr/local/bin/ so the installer never executes an unverified binary.
---
Duplicate comments:
In @.github/workflows/e2e-slow.yml:
- Line 43: Replace the floating action tags with immutable commit SHAs for both
erlef/setup-beam@v1 and iFaxity/wait-on-action@v1.2.1 in the workflow: locate
the uses entries referencing "erlef/setup-beam@v1" and
"iFaxity/wait-on-action@v1.2.1" and swap the tag names for the corresponding
full commit SHA from each action's repository (use the action repo's git refs to
obtain the exact 40-character SHA), then update the workflow to use e.g.
erlef/setup-beam@<sha> and iFaxity/wait-on-action@<sha> so the actions are
pinned immutably.
- Line 8: The pull_request event's types array incorrectly contains the
unsupported activity "push" (see the entry "types: [opened, synchronize,
reopened, labeled, unlabeled, push]"); remove "push" from the pull_request.types
list so it only contains valid activities (e.g., opened, synchronize, reopened,
labeled, unlabeled) to pass workflow validation.
In @.github/workflows/e2e.yml:
- Line 51: The workflow uses mutable action refs erlef/setup-beam@v1 and
iFaxity/wait-on-action@v1.2.1 which should be pinned to immutable commit SHAs;
update each uses: reference to the full commit SHA for erlef/setup-beam and for
iFaxity/wait-on-action (replace the `@v1` / `@v1.2.1` suffix with @<commit-sha>) so
the workflow always runs a fixed, auditable version, and ensure you replace both
occurrences (including the other occurrence noted by the reviewer).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ef68ff1b-559e-434e-a420-209473dc0b14
📒 Files selected for processing (2)
.github/workflows/e2e-slow.yml.github/workflows/e2e.yml
Comment on lines
+48
to
+53
| - name: Download ngrok | ||
| run: | | ||
| wget -q -O ngrok.tgz https://bin.ngrok.com/c/bNyj1mQVY4c/ngrok-v3-stable-linux-amd64.tgz | ||
| tar -xzvf ngrok.tgz | ||
| chmod +x ngrok | ||
| sudo mv ngrok /usr/local/bin/ |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'ngrok-v3-stable-linux-amd64.tgz|sha256sum|gpg|cosign' .github/workflows/e2e-slow.ymlRepository: glific/cypress-testing
Length of output: 167
Verify ngrok binary integrity before execution.
This workflow downloads and executes ngrok without checksum or signature verification. Add checksum validation or use signed binaries to ensure the downloaded binary hasn't been tampered with.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/e2e-slow.yml around lines 48 - 53, Add integrity
verification for the "Download ngrok" step: fetch the official checksum or
signature alongside the ngrok archive, verify the downloaded file (e.g., compare
sha256 checksum or verify GPG signature) before making it executable and moving
it to /usr/local/bin; if verification fails, exit the job with a clear error.
Ensure verification happens after downloading ngrok.tgz (the file produced by
wget -q -O ngrok.tgz https://... ) and before chmod +x ngrok and sudo mv ngrok
/usr/local/bin/, and fail fast on mismatch so a tampered binary is never
executed or installed.
Comment on lines
+62
to
+80
| - name: Set GLIFIC_API_HOST_OVERRIDE env | ||
| run: | | ||
| NGROK_API_URL="http://127.0.0.1:4040/api/tunnels" | ||
| count=0 | ||
| until curl -s "${NGROK_API_URL}" | grep -q '"public_url"' || [ $count -eq 10 ]; do | ||
| echo "ngrok public_url not ready" | ||
| sleep 2 | ||
| count=$((count+1)) | ||
| done | ||
| API_URL=$(curl -s ${NGROK_API_URL} | jq -r '.tunnels[] | select(.proto=="https") | .public_url') | ||
| if [ -z "$API_URL" ]; then | ||
| API_URL=$(curl -s ${NGROK_API_URL} | jq -r '.tunnels[] | select(.proto=="http") | .public_url') | ||
| fi | ||
| echo "ngrok tunnels" | ||
| curl -s http://127.0.0.1:4040/api/tunnels | ||
| echo "api_url=$API_URL" | ||
|
|
||
| echo "GLIFIC_API_HOST_OVERRIDE=${API_URL}" >> $GITHUB_ENV | ||
|
|
There was a problem hiding this comment.
Guard against empty/null API_URL before exporting it.
If tunnel discovery fails, Line [79] still writes GLIFIC_API_HOST_OVERRIDE, causing downstream backend setup instability.
Based on learnings: GLIFIC_API_HOST_OVERRIDE is consumed by backend setup/server steps, so validating it here is required for reliable E2E execution.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/e2e-slow.yml around lines 62 - 80, The step that computes
API_URL from NGROK_API_URL may leave API_URL empty but still exports
GLIFIC_API_HOST_OVERRIDE; update the step to validate API_URL (the variable
produced by jq from NGROK_API_URL) before appending to $GITHUB_ENV: if API_URL
is empty or null, fail the job with a clear error and non-zero exit (or
optionally set a safe default) instead of writing an empty
GLIFIC_API_HOST_OVERRIDE, and log the NGROK_API_URL/curl output for debugging;
make this change in the block that sets API_URL and writes
GLIFIC_API_HOST_OVERRIDE so downstream backend setup/server steps don’t receive
an empty value.
Comment on lines
+56
to
+62
| - name: Download ngrok | ||
| run: | | ||
| wget -q -O ngrok.tgz https://bin.ngrok.com/c/bNyj1mQVY4c/ngrok-v3-stable-linux-amd64.tgz | ||
| tar -xzvf ngrok.tgz | ||
| chmod +x ngrok | ||
| sudo mv ngrok /usr/local/bin/ | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'ngrok-v3-stable-linux-amd64.tgz|sha256sum|gpg|cosign' .github/workflows/e2e.ymlRepository: glific/cypress-testing
Length of output: 167
🏁 Script executed:
cat -n .github/workflows/e2e.yml | sed -n '50,70p'Repository: glific/cypress-testing
Length of output: 962
🏁 Script executed:
rg -n 'secret|checksum|verify|signature|integrity' .github/workflows/e2e.ymlRepository: glific/cypress-testing
Length of output: 874
Verify ngrok binary integrity before installing it.
The ngrok binary is downloaded without checksum or signature validation and then executed with sudo privileges. This creates a supply-chain risk—if the download is intercepted or the upstream source is compromised, a malicious binary could gain full system access.
Add SHA256 checksum verification:
Hardening example
- name: Download ngrok
run: |
wget -q -O ngrok.tgz https://bin.ngrok.com/c/bNyj1mQVY4c/ngrok-v3-stable-linux-amd64.tgz
+ echo "${{ secrets.NGROK_TGZ_SHA256 }} ngrok.tgz" | sha256sum -c -
tar -xzvf ngrok.tgz
chmod +x ngrok
sudo mv ngrok /usr/local/bin/Store the hash as a repository secret.
📝 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
| - name: Download ngrok | |
| run: | | |
| wget -q -O ngrok.tgz https://bin.ngrok.com/c/bNyj1mQVY4c/ngrok-v3-stable-linux-amd64.tgz | |
| tar -xzvf ngrok.tgz | |
| chmod +x ngrok | |
| sudo mv ngrok /usr/local/bin/ | |
| - name: Download ngrok | |
| run: | | |
| wget -q -O ngrok.tgz https://bin.ngrok.com/c/bNyj1mQVY4c/ngrok-v3-stable-linux-amd64.tgz | |
| echo "${{ secrets.NGROK_TGZ_SHA256 }} ngrok.tgz" | sha256sum -c - | |
| tar -xzvf ngrok.tgz | |
| chmod +x ngrok | |
| sudo mv ngrok /usr/local/bin/ | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/e2e.yml around lines 56 - 62, Add SHA256 checksum
verification for the "Download ngrok" step: fetch the ngrok tarball as before
but also fetch or read the expected SHA256 hash from a repository secret (e.g.,
NGROK_SHA256) and compute the downloaded file's checksum with sha256sum (or
shasum -a 256), then compare and fail the workflow if they differ before making
the file executable or running sudo mv; ensure the verification happens prior to
chmod +x ngrok and sudo mv ngrok /usr/local/bin/ so the installer never executes
an unverified binary.
Comment on lines
+71
to
+91
| - name: Set GLIFIC_API_HOST_OVERRIDE env | ||
| run: | | ||
| # Get public URL from ngrok API | ||
| NGROK_API_URL="http://127.0.0.1:4040/api/tunnels" | ||
| count=0 | ||
| until curl -s "${NGROK_API_URL}" | grep -q '"public_url"' || [ $count -eq 10 ]; do | ||
| echo "ngrok public_url not ready" | ||
| sleep 2 | ||
| count=$((count+1)) | ||
| done | ||
| API_URL=$(curl -s ${NGROK_API_URL} | jq -r '.tunnels[] | select(.proto=="https") | .public_url') | ||
| # Fall back to http if https not available | ||
| if [ -z "$API_URL" ]; then | ||
| API_URL=$(curl -s ${NGROK_API_URL} | jq -r '.tunnels[] | select(.proto=="http") | .public_url') | ||
| fi | ||
| # For debugging | ||
| echo "ngrok tunnels" | ||
| curl -s http://127.0.0.1:4040/api/tunnels | ||
| echo "api_url=$API_URL" | ||
|
|
||
| echo "GLIFIC_API_HOST_OVERRIDE=${API_URL}" >> $GITHUB_ENV |
There was a problem hiding this comment.
Fail fast if ngrok tunnel URL cannot be resolved.
API_URL can remain empty/null after retries, but Line [91] still exports it. That creates non-deterministic backend setup failures later.
Suggested fix
- name: Set GLIFIC_API_HOST_OVERRIDE env
run: |
@@
- API_URL=$(curl -s ${NGROK_API_URL} | jq -r '.tunnels[] | select(.proto=="https") | .public_url')
+ API_URL=$(curl -s "${NGROK_API_URL}" | jq -r '.tunnels[] | select(.proto=="https") | .public_url')
@@
- API_URL=$(curl -s ${NGROK_API_URL} | jq -r '.tunnels[] | select(.proto=="http") | .public_url')
+ API_URL=$(curl -s "${NGROK_API_URL}" | jq -r '.tunnels[] | select(.proto=="http") | .public_url')
fi
+ if [ -z "$API_URL" ] || [ "$API_URL" = "null" ]; then
+ echo "::error::Could not resolve ngrok public URL"
+ tail -n 200 ngrok.log || true
+ exit 1
+ fi
@@
echo "GLIFIC_API_HOST_OVERRIDE=${API_URL}" >> $GITHUB_ENVBased on learnings: GLIFIC_API_HOST_OVERRIDE is intended for backend commands (mix setup / mix phx.server), so this check protects that flow directly.
📝 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
| - name: Set GLIFIC_API_HOST_OVERRIDE env | |
| run: | | |
| # Get public URL from ngrok API | |
| NGROK_API_URL="http://127.0.0.1:4040/api/tunnels" | |
| count=0 | |
| until curl -s "${NGROK_API_URL}" | grep -q '"public_url"' || [ $count -eq 10 ]; do | |
| echo "ngrok public_url not ready" | |
| sleep 2 | |
| count=$((count+1)) | |
| done | |
| API_URL=$(curl -s ${NGROK_API_URL} | jq -r '.tunnels[] | select(.proto=="https") | .public_url') | |
| # Fall back to http if https not available | |
| if [ -z "$API_URL" ]; then | |
| API_URL=$(curl -s ${NGROK_API_URL} | jq -r '.tunnels[] | select(.proto=="http") | .public_url') | |
| fi | |
| # For debugging | |
| echo "ngrok tunnels" | |
| curl -s http://127.0.0.1:4040/api/tunnels | |
| echo "api_url=$API_URL" | |
| echo "GLIFIC_API_HOST_OVERRIDE=${API_URL}" >> $GITHUB_ENV | |
| - name: Set GLIFIC_API_HOST_OVERRIDE env | |
| run: | | |
| # Get public URL from ngrok API | |
| NGROK_API_URL="http://127.0.0.1:4040/api/tunnels" | |
| count=0 | |
| until curl -s "${NGROK_API_URL}" | grep -q '"public_url"' || [ $count -eq 10 ]; do | |
| echo "ngrok public_url not ready" | |
| sleep 2 | |
| count=$((count+1)) | |
| done | |
| API_URL=$(curl -s "${NGROK_API_URL}" | jq -r '.tunnels[] | select(.proto=="https") | .public_url') | |
| # Fall back to http if https not available | |
| if [ -z "$API_URL" ]; then | |
| API_URL=$(curl -s "${NGROK_API_URL}" | jq -r '.tunnels[] | select(.proto=="http") | .public_url') | |
| fi | |
| if [ -z "$API_URL" ] || [ "$API_URL" = "null" ]; then | |
| echo "::error::Could not resolve ngrok public URL" | |
| tail -n 200 ngrok.log || true | |
| exit 1 | |
| fi | |
| # For debugging | |
| echo "ngrok tunnels" | |
| curl -s http://127.0.0.1:4040/api/tunnels | |
| echo "api_url=$API_URL" | |
| echo "GLIFIC_API_HOST_OVERRIDE=${API_URL}" >> $GITHUB_ENV |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/e2e.yml around lines 71 - 91, The step that reads ngrok
tunnels (NGROK_API_URL) may leave API_URL empty and still write
GLIFIC_API_HOST_OVERRIDE, causing downstream nondeterministic failures; after
the existing retries and selection logic for API_URL, add a guard that checks if
API_URL is empty and if so echo a clear error (including the ngrok API output
for debugging) and exit 1 so the workflow fails fast, and only append
"GLIFIC_API_HOST_OVERRIDE=${API_URL}" to $GITHUB_ENV when API_URL is non-empty;
reference the variables/API logic named NGROK_API_URL, API_URL and the env key
GLIFIC_API_HOST_OVERRIDE to locate where to add this check.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Filesearch tests were mocking requests to Glific BE which made these unit tests and not integrations tests. This PR removes those mocks and also connects to Kaapi staging to make this a proper E2E test. Other changes to make this work -
Summary by CodeRabbit
Tests
Documentation
Chores