Skip to content

migration test fixes - #4787

Merged
akshaydeo merged 1 commit into
mainfrom
06-30-migration_test_fixes
Jun 29, 2026
Merged

migration test fixes#4787
akshaydeo merged 1 commit into
mainfrom
06-30-migration_test_fixes

Conversation

@akshaydeo

Copy link
Copy Markdown
Contributor

Summary

Briefly explain the purpose of this PR and the problem it solves.

Changes

  • What was changed and why
  • Any notable design decisions or trade-offs

Type of change

  • Bug fix
  • Feature
  • Refactor
  • Documentation
  • Chore/CI

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (React)
  • Docs

How to test

Describe the steps to validate this change. Include commands and expected outcomes.

# Core/Transports
go version
go test ./...

# UI
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build

If adding new configs or environment variables, document them here.

Screenshots/Recordings

If UI changes, add before/after screenshots or short clips.

Breaking changes

  • Yes
  • No

If yes, describe impact and migration instructions.

Related issues

Link related issues and discussions. Example: Closes #123

Security considerations

Note any security implications (auth, secrets, PII, sandboxing, etc.).

Checklist

  • I read docs/contributing/README.md and followed the guidelines
  • I added/updated tests where appropriate
  • I updated documentation where needed
  • I verified builds succeed (Go and UI)
  • I verified the CI pipeline passes locally if applicable

@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: d6938902-fabf-44df-9fdc-958a159a300e

📥 Commits

Reviewing files that changed from the base of the PR and between d4d9b30 and f25a192.

📒 Files selected for processing (1)
  • .github/workflows/scripts/run-migration-tests.sh

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Improved migration test coverage for a configuration setting so generated test data stays consistent across databases.
    • Added handling for an additional config field in test data generation when that field is present.

Walkthrough

The migration test script gains two conditional UPDATE blocks — one each for PostgreSQL and SQLite — that set config_client.dump_errors_in_console_logs to false/0 for id = 1 when the column exists during dynamic column population.

Changes

Migration Test Script

Layer / File(s) Summary
Dynamic column updaters for dump_errors_in_console_logs
.github/workflows/scripts/run-migration-tests.sh
Both append_dynamic_columns_postgres and append_dynamic_columns_sqlite receive a conditional block that checks for the config_client.dump_errors_in_console_logs column and emits an UPDATE setting it to false/0 for config_client.id = 1.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Suggested reviewers

  • danpiths

Poem

🐇 A column appears, the migration must know,
Set errors to false, let the test data flow,
Postgres says false, SQLite says 0,
The rabbit hops on — the coverage grows!

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 06-30-migration_test_fixes

Comment @coderabbitai help to get the list of available commands.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link
Copy Markdown
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@akshaydeo
akshaydeo marked this pull request as ready for review June 29, 2026 21:21
@akshaydeo
akshaydeo requested a review from a team as a code owner June 29, 2026 21:21
@mintlify

mintlify Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
Bifrost 🟡 Building Jun 29, 2026, 9:21 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@akshaydeo
akshaydeo merged commit 57d9f64 into main Jun 29, 2026
11 of 14 checks passed
@akshaydeo
akshaydeo deleted the 06-30-migration_test_fixes branch June 29, 2026 21:22
@coderabbitai
coderabbitai Bot requested a review from danpiths June 29, 2026 21:22
@greptile-apps

greptile-apps Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 5/5

The change is a two-hunk addition to a CI test helper script with no impact on production code paths.

Both additions correctly mirror the pattern used for every other dynamically-added column: the PostgreSQL path uses the false boolean literal and the SQLite path uses the integer 0, both behind their respective column_exists_* guards. The SQLite entry sits inside the existing if [ -f "$config_db" ] outer guard, which is the right place for config-store column seeds. No logic, schema, or test assertions are altered beyond registering the new column.

No files require special attention.

Important Files Changed

Filename Overview
.github/workflows/scripts/run-migration-tests.sh Adds guarded seed entries for the new v1.6.0 dump_errors_in_console_logs boolean column in both the PostgreSQL and SQLite dynamic-column append functions, correctly using false/0 to match each engine's boolean representation.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[append_dynamic_columns] --> B{Database Type}
    B -->|PostgreSQL| C[column_exists_postgres\nconfig_client\ndump_errors_in_console_logs]
    B -->|SQLite| D{config_db file exists?}
    D -->|Yes| E[column_exists_sqlite\nconfig_client\ndump_errors_in_console_logs]
    D -->|No| F[Skip]
    C -->|Exists| G["UPDATE config_client\nSET dump_errors_in_console_logs = false\nWHERE id = 1"]
    C -->|Not Exists| H[Skip]
    E -->|Exists| I["UPDATE config_client\nSET dump_errors_in_console_logs = 0\nWHERE id = 1"]
    E -->|Not Exists| J[Skip]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[append_dynamic_columns] --> B{Database Type}
    B -->|PostgreSQL| C[column_exists_postgres\nconfig_client\ndump_errors_in_console_logs]
    B -->|SQLite| D{config_db file exists?}
    D -->|Yes| E[column_exists_sqlite\nconfig_client\ndump_errors_in_console_logs]
    D -->|No| F[Skip]
    C -->|Exists| G["UPDATE config_client\nSET dump_errors_in_console_logs = false\nWHERE id = 1"]
    C -->|Not Exists| H[Skip]
    E -->|Exists| I["UPDATE config_client\nSET dump_errors_in_console_logs = 0\nWHERE id = 1"]
    E -->|Not Exists| J[Skip]
Loading

Reviews (1): Last reviewed commit: "migration test fixes" | Re-trigger Greptile

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants