test: added workflow to test post-install patch on fresh site#3903
test: added workflow to test post-install patch on fresh site#3903ljain112 wants to merge 7 commits intoresilient-tech:developfrom
Conversation
|
Warning Rate limit exceeded@ljain112 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 12 minutes and 2 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds infrastructure for post-install testing in a Frappé/BENCHE project: a Bash script orchestrating environment setup, system dependencies, database provisioning, and application bootstrap; a GitHub Actions workflow triggering the tests; and helper functions for India Compliance test data initialization. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In @.github/helper/create_test_data.py:
- Line 44: Suppress the semgrep warning for the intentional manual commit by
adding a nosemgrep directive and brief explanation immediately above the
frappe.db.commit() call in create_test_data.py; e.g., add a single-line comment
referencing the commit line (frappe.db.commit()) that includes a nosemgrep
pragma for the relevant rule and a short justification like "standalone script,
manual commit required" so the pipeline ignores this intentional commit.
In @.github/helper/post_install.sh:
- Around line 21-25: Rename the function install_whktml to install_wkhtmltopdf
and update its invocation to install_wkhtmltopdf &, and make the apt call
non-interactive by adding the -y flag (e.g., sudo apt install -y
/tmp/wkhtmltox.deb); keep the wget usage the same but ensure the function name
and background caller match the corrected identifier.
- Around line 16-19: The apt commands in post_install.sh (the lines running
"sudo apt update", "sudo apt remove mysql-server mysql-client", and "sudo apt
install libcups2-dev redis-server mariadb-client") are missing non-interactive
flags; update them to run non-interactively by adding the -y flag to apt remove
and apt install (and optionally apt update as "sudo apt update -y"), and
consider prefixing with DEBIAN_FRONTEND=noninteractive if needed in CI to
suppress prompts.
- Around line 62-63: The backgrounded "bench start &" can race with the
immediate "bench reinstall --yes"; change this to start bench in background then
wait for service readiness before running reinstall—implement a short wait loop
with timeout that probes the app (e.g., curl http://localhost:8000/health or
checks the expected port/socket) or a simple retry sleep (e.g., sleep 1 in a
loop) until a successful response, then run "bench reinstall --yes"; keep the
timeout/failure path to exit nonzero if services never become healthy.
In @.github/workflows/post-install-tests.yml:
- Around line 105-107: The workflow shows ~/frappe-bench/bench_start.log but
post_install.sh starts the bench with `bench start &` without redirection so the
log is never created; update post_install.sh to start the bench with its
stdout/stderr redirected into ~/frappe-bench/bench_start.log (e.g., use nohup or
append `> ~/frappe-bench/bench_start.log 2>&1 &`), ensure the ~/frappe-bench
directory exists before writing, and keep the background start command (bench
start) otherwise unchanged so the CI step can cat the log.
🧹 Nitpick comments (2)
.github/helper/create_test_data.py (1)
8-8: Minor: Extra space in import statement.There's a double space before
setup_complete.Suggested fix
-from frappe.desk.page.setup_wizard.setup_wizard import setup_complete +from frappe.desk.page.setup_wizard.setup_wizard import setup_complete.github/workflows/post-install-tests.yml (1)
89-89: Quote shell variables to prevent word splitting.Static analysis flagged unquoted variables. While these are typically safe in GitHub Actions, quoting is good practice.
Suggested fix
- run: echo "dir=$(yarn cache dir)" >> $GITHUB_OUTPUT + run: echo "dir=$(yarn cache dir)" >> "$GITHUB_OUTPUT"- bash ${GITHUB_WORKSPACE}/.github/helper/post_install.sh + bash "${GITHUB_WORKSPACE}/.github/helper/post_install.sh"Also applies to: 100-101
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/helper/create_test_data.py.github/helper/post_install.sh.github/workflows/post-install-tests.yml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-26T10:26:32.848Z
Learnt from: karm1000
Repo: resilient-tech/india-compliance PR: 3857
File: .github/workflows/server-tests.yml:48-48
Timestamp: 2025-12-26T10:26:32.848Z
Learning: CI workflows related to server tests should reflect supported environment versions. Since Frappe framework v16 (develop branch) uses MariaDB 11.8, ensure workflows under .github/workflows (e.g., server-tests.yml) test against MariaDB 11.8 by updating the test matrix, Docker compose services, or container images (e.g., mariadb:11.8) and verify compatibility in tests.
Applied to files:
.github/workflows/post-install-tests.yml
🧬 Code graph analysis (1)
.github/helper/post_install.sh (1)
india_compliance/gst_india/overrides/transaction.py (1)
update(1154-1167)
🪛 actionlint (1.7.10)
.github/workflows/post-install-tests.yml
89-89: shellcheck reported issue in this script: SC2086:info:1:33: Double quote to prevent globbing and word splitting
(shellcheck)
100-100: shellcheck reported issue in this script: SC2086:info:1:6: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 GitHub Actions: Linters
.github/helper/create_test_data.py
[error] 44-44: Blocking code rule fired: frappe-semgrep-rules.rules.frappe-manual-commit. Manually committing a transaction is highly discouraged. If manual commit is required, add an explanation and a '# nosemgrep' on the same line.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Python Unit Tests
- GitHub Check: Post Install Patches
- GitHub Check: Mergify Merge Protections
- GitHub Check: Summary
🔇 Additional comments (2)
.github/helper/post_install.sh (1)
65-74: Installation sequence looks correct.The workflow correctly:
- Gets India Compliance after reinstall (so it's not installed during reinstall)
- Creates India company data before installing India Compliance
- Then installs India Compliance to trigger post-install patches
This ensures post-install patches run against existing India company data.
.github/workflows/post-install-tests.yml (1)
39-46: MariaDB 11.8 is correctly configured.The MariaDB service version aligns with Frappe framework v16 requirements. Based on learnings, this is the correct version for the develop branch.
|
|
||
|
|
||
|
|
||
| frappe.db.commit() |
There was a problem hiding this comment.
Add nosemgrep directive to address pipeline failure.
The semgrep rule flags manual commits, but this is a standalone script without request context—manual commit is intentional and required here. Add an explanation and suppress the warning.
Suggested fix
- frappe.db.commit()
+ frappe.db.commit() # nosemgrep: standalone script requires explicit commit📝 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.
| frappe.db.commit() | |
| frappe.db.commit() # nosemgrep: standalone script requires explicit commit |
🧰 Tools
🪛 GitHub Actions: Linters
[error] 44-44: Blocking code rule fired: frappe-semgrep-rules.rules.frappe-manual-commit. Manually committing a transaction is highly discouraged. If manual commit is required, add an explanation and a '# nosemgrep' on the same line.
🤖 Prompt for AI Agents
In @.github/helper/create_test_data.py at line 44, Suppress the semgrep warning
for the intentional manual commit by adding a nosemgrep directive and brief
explanation immediately above the frappe.db.commit() call in
create_test_data.py; e.g., add a single-line comment referencing the commit line
(frappe.db.commit()) that includes a nosemgrep pragma for the relevant rule and
a short justification like "standalone script, manual commit required" so the
pipeline ignores this intentional commit.
| install_whktml() { | ||
| wget -O /tmp/wkhtmltox.deb https://github.com/wkhtmltopdf/packaging/releases/download/0.12.6.1-2/wkhtmltox_0.12.6.1-2.jammy_amd64.deb | ||
| sudo apt install /tmp/wkhtmltox.deb | ||
| } | ||
| install_whktml & |
There was a problem hiding this comment.
Typo in function name and missing -y flag.
The function name has a typo (whktml instead of wkhtmltopdf), and the apt install inside needs -y for non-interactive execution.
Suggested fix
-install_whktml() {
+install_wkhtmltopdf() {
wget -O /tmp/wkhtmltox.deb https://github.com/wkhtmltopdf/packaging/releases/download/0.12.6.1-2/wkhtmltox_0.12.6.1-2.jammy_amd64.deb
- sudo apt install /tmp/wkhtmltox.deb
+ sudo apt install -y /tmp/wkhtmltox.deb
}
-install_whktml &
+install_wkhtmltopdf &📝 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.
| install_whktml() { | |
| wget -O /tmp/wkhtmltox.deb https://github.com/wkhtmltopdf/packaging/releases/download/0.12.6.1-2/wkhtmltox_0.12.6.1-2.jammy_amd64.deb | |
| sudo apt install /tmp/wkhtmltox.deb | |
| } | |
| install_whktml & | |
| install_wkhtmltopdf() { | |
| wget -O /tmp/wkhtmltox.deb https://github.com/wkhtmltopdf/packaging/releases/download/0.12.6.1-2/wkhtmltox_0.12.6.1-2.jammy_amd64.deb | |
| sudo apt install -y /tmp/wkhtmltox.deb | |
| } | |
| install_wkhtmltopdf & |
🤖 Prompt for AI Agents
In @.github/helper/post_install.sh around lines 21 - 25, Rename the function
install_whktml to install_wkhtmltopdf and update its invocation to
install_wkhtmltopdf &, and make the apt call non-interactive by adding the -y
flag (e.g., sudo apt install -y /tmp/wkhtmltox.deb); keep the wget usage the
same but ensure the function name and background caller match the corrected
identifier.
| - name: Show bench output | ||
| if: ${{ always() }} | ||
| run: cat ~/frappe-bench/bench_start.log || true |
There was a problem hiding this comment.
bench_start.log is never created.
The workflow tries to display ~/frappe-bench/bench_start.log, but post_install.sh runs bench start & without redirecting output to this file. The log file won't exist.
Update post_install.sh to redirect bench output:
-bench start &
+bench start > ~/frappe-bench/bench_start.log 2>&1 &🤖 Prompt for AI Agents
In @.github/workflows/post-install-tests.yml around lines 105 - 107, The
workflow shows ~/frappe-bench/bench_start.log but post_install.sh starts the
bench with `bench start &` without redirection so the log is never created;
update post_install.sh to start the bench with its stdout/stderr redirected into
~/frappe-bench/bench_start.log (e.g., use nohup or append `>
~/frappe-bench/bench_start.log 2>&1 &`), ensure the ~/frappe-bench directory
exists before writing, and keep the background start command (bench start)
otherwise unchanged so the CI step can cat the log.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.github/helper/post_install.sh:
- Around line 16-19: Update the non-interactive install/remove commands to
include the automatic yes flag so CI won't block: modify the apt remove
invocation ("apt remove mysql-server mysql-client") and the apt install
invocation ("apt install libcups2-dev redis-server mariadb-client") to include
"-y" (and consider using DEBIAN_FRONTEND=noninteractive if further
non-interactive behavior is needed).
- Around line 61-63: The script runs "bench start &" then immediately runs
"bench reinstall --yes", causing a race; modify the post_install.sh sequence so
that after invoking bench start (the "bench start &" background job) you block
until the bench server is ready before running "bench reinstall --yes" —
implement a readiness check (e.g., poll an HTTP health endpoint such as a
frappe/bench API or curl localhost:8000/api/method/frappe.ping until it returns
success, or loop checking the port is open) with a timeout and error handling,
then proceed to run "bench reinstall --yes"; ensure you reference the existing
commands ("bench start &", "bench reinstall --yes", and "bench use test_site")
when updating the script.
- Around line 21-26: Rename the function install_whktml to install_wkhtml (and
update its invocation) to fix the typo, and make the apt install call
non-interactive by adding the -y flag (i.e., change the command inside
install_wkhtml to use sudo apt install -y /tmp/wkhtmltox.deb); ensure the
background call and wkpid=$! still reference the corrected function name.
🧹 Nitpick comments (1)
.github/helper/post_install.sh (1)
51-54: Consider anchoring sed patterns to line start.The current patterns work but could be more precise by anchoring to the start of line to avoid unintended matches elsewhere in the file.
Optional improvement
-sed -i 's/watch:/# watch:/g' Procfile -sed -i 's/schedule:/# schedule:/g' Procfile -sed -i 's/socketio:/# socketio:/g' Procfile -sed -i 's/redis_socketio:/# redis_socketio:/g' Procfile +sed -i 's/^watch:/# watch:/g' Procfile +sed -i 's/^schedule:/# schedule:/g' Procfile +sed -i 's/^socketio:/# socketio:/g' Procfile +sed -i 's/^redis_socketio:/# redis_socketio:/g' Procfile
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/helper/post_install.sh
🧰 Additional context used
🧬 Code graph analysis (1)
.github/helper/post_install.sh (1)
india_compliance/gst_india/overrides/transaction.py (1)
update(1154-1167)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Post Install Patches
- GitHub Check: Python Unit Tests
- GitHub Check: Mergify Merge Protections
- GitHub Check: Summary
🔇 Additional comments (4)
.github/helper/post_install.sh (4)
1-10: LGTM!Good use of
set -efor early failure and pre-flight checks for Python syntax validation and merge conflict detection.
28-35: LGTM!The bench initialization and site setup steps are appropriate for CI. The use of
--skip-assetsand--depth 1helps speed up the workflow.
38-47: LGTM!Database provisioning with test credentials is appropriate for CI. UTF-8 mb4 configuration is correct for Frappe/ERPNext compatibility.
65-78: LGTM!The India Compliance workflow is well-documented with clear comments explaining the intentional ordering:
- Get app after reinstall (so it's not installed during reinstall)
- Create test data before installing (to simulate existing data scenario)
- Install with verbose output for CI debugging
This correctly sets up the post-install patch testing scenario.
| install_whktml() { | ||
| wget -O /tmp/wkhtmltox.deb https://github.com/wkhtmltopdf/packaging/releases/download/0.12.6.1-2/wkhtmltox_0.12.6.1-2.jammy_amd64.deb | ||
| sudo apt install /tmp/wkhtmltox.deb | ||
| } | ||
| install_whktml & | ||
| wkpid=$! |
There was a problem hiding this comment.
Fix typo in function name and add -y flag.
The function name install_whktml appears to be a typo. Also, the apt install inside needs -y for non-interactive execution.
Proposed fix
-install_whktml() {
+install_wkhtmltox() {
wget -O /tmp/wkhtmltox.deb https://github.com/wkhtmltopdf/packaging/releases/download/0.12.6.1-2/wkhtmltox_0.12.6.1-2.jammy_amd64.deb
- sudo apt install /tmp/wkhtmltox.deb
+ sudo apt install -y /tmp/wkhtmltox.deb
}
-install_whktml &
+install_wkhtmltox &📝 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.
| install_whktml() { | |
| wget -O /tmp/wkhtmltox.deb https://github.com/wkhtmltopdf/packaging/releases/download/0.12.6.1-2/wkhtmltox_0.12.6.1-2.jammy_amd64.deb | |
| sudo apt install /tmp/wkhtmltox.deb | |
| } | |
| install_whktml & | |
| wkpid=$! | |
| install_wkhtmltox() { | |
| wget -O /tmp/wkhtmltox.deb https://github.com/wkhtmltopdf/packaging/releases/download/0.12.6.1-2/wkhtmltox_0.12.6.1-2.jammy_amd64.deb | |
| sudo apt install -y /tmp/wkhtmltox.deb | |
| } | |
| install_wkhtmltox & | |
| wkpid=$! |
🤖 Prompt for AI Agents
In @.github/helper/post_install.sh around lines 21 - 26, Rename the function
install_whktml to install_wkhtml (and update its invocation) to fix the typo,
and make the apt install call non-interactive by adding the -y flag (i.e.,
change the command inside install_wkhtml to use sudo apt install -y
/tmp/wkhtmltox.deb); ensure the background call and wkpid=$! still reference the
corrected function name.
| bench use test_site | ||
| bench start & | ||
| bench reinstall --yes |
There was a problem hiding this comment.
Potential race condition: bench reinstall may run before bench is ready.
bench start & spawns the server in the background, but bench reinstall --yes executes immediately without waiting for the server to be fully initialized. This could cause intermittent failures.
Proposed fix: Add a delay or readiness check
bench use test_site
bench start &
+
+# Wait for bench to be ready
+sleep 10
+
bench reinstall --yesAlternatively, consider using a more robust readiness check:
bench use test_site
bench start &
+
+# Wait for bench to be ready
+until curl -s http://localhost:8000 > /dev/null 2>&1; do
+ echo "Waiting for bench to start..."
+ sleep 2
+done
+
bench reinstall --yes📝 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.
| bench use test_site | |
| bench start & | |
| bench reinstall --yes | |
| bench use test_site | |
| bench start & | |
| # Wait for bench to be ready | |
| sleep 10 | |
| bench reinstall --yes |
🤖 Prompt for AI Agents
In @.github/helper/post_install.sh around lines 61 - 63, The script runs "bench
start &" then immediately runs "bench reinstall --yes", causing a race; modify
the post_install.sh sequence so that after invoking bench start (the "bench
start &" background job) you block until the bench server is ready before
running "bench reinstall --yes" — implement a readiness check (e.g., poll an
HTTP health endpoint such as a frappe/bench API or curl
localhost:8000/api/method/frappe.ping until it returns success, or loop checking
the port is open) with a timeout and error handling, then proceed to run "bench
reinstall --yes"; ensure you reference the existing commands ("bench start &",
"bench reinstall --yes", and "bench use test_site") when updating the script.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @.github/helper/post_install.sh:
- Line 23: The apt install command in the background installer currently runs
interactively ("sudo apt install /tmp/wkhtmltox.deb"); update that invocation to
be non-interactive by adding the -y flag (change to "sudo apt install -y
/tmp/wkhtmltox.deb") and apply the same -y flag to any other apt install calls
inside the same background function so the script can run unattended.
- Around line 16-19: Update the apt commands to run non-interactively: modify
the commands "sudo apt remove mysql-server mysql-client" and "sudo apt install
libcups2-dev redis-server mariadb-client" to include the -y flag (e.g., "sudo
apt remove -y ..." and "sudo apt install -y ...") so the script won’t prompt
during CI runs.
- Around line 62-63: The script currently backgrounds "bench start &" and
immediately runs "bench reinstall --yes", which can race before services are
ready; change it to wait for the Bench web/worker processes to become healthy
before running reinstall by replacing the immediate call with a readiness check
(e.g., poll a health endpoint or wait for a PID/port) after launching "bench
start" and only then execute "bench reinstall --yes"; reference the launched
command "bench start &" and the subsequent "bench reinstall --yes" when updating
the script so the reinstall runs after a successful health check or short
timeout.
In @india_compliance/tests/__init__.py:
- Around line 24-52: The test GSTIN in setup_india_company is 14 chars and will
be rejected; update setup_args["company_gstin"] to a valid 15-character GSTIN
(e.g., follow the pattern used by the validation: 2-digit state code + 10-char
PAN + entity code + 'Z' + checksum) so the value remains set during setup;
locate the setup_india_company function and change the "company_gstin" entry in
the setup_args dict to a correct 15-character GSTIN string.
🧹 Nitpick comments (2)
.github/helper/post_install.sh (2)
21-26: Minor: Typo in function nameinstall_whktml.The function name has a typo — should be
install_wkhtmltopdfor similar for clarity. Also, the background process lacks error handling; if the download or install fails, the script will continue and only fail later atwait $wkpid.Optional: Improve naming and add error handling
-install_whktml() { +install_wkhtmltopdf() { wget -O /tmp/wkhtmltox.deb https://github.com/wkhtmltopdf/packaging/releases/download/0.12.6.1-2/wkhtmltox_0.12.6.1-2.jammy_amd64.deb sudo apt install /tmp/wkhtmltox.deb } -install_whktml & +install_wkhtmltopdf & wkpid=$!At line 59, consider adding error handling:
-wait $wkpid +wait $wkpid || { echo "wkhtmltopdf installation failed"; exit 1; }
51-54: Consider using more precisesedpatterns to avoid unintended matches.The current
sedpatterns will replace the first occurrence of each service name. If the Procfile has comments or other lines containing these words, they might be affected. Using anchored patterns would be safer.Optional: More precise sed patterns
-sed -i 's/watch:/# watch:/g' Procfile -sed -i 's/schedule:/# schedule:/g' Procfile -sed -i 's/socketio:/# socketio:/g' Procfile -sed -i 's/redis_socketio:/# redis_socketio:/g' Procfile +sed -i 's/^watch:/# watch:/g' Procfile +sed -i 's/^schedule:/# schedule:/g' Procfile +sed -i 's/^socketio:/# socketio:/g' Procfile +sed -i 's/^redis_socketio:/# redis_socketio:/g' Procfile
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/helper/post_install.shindia_compliance/tests/__init__.py
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: vorasmit
Repo: resilient-tech/india-compliance PR: 3399
File: india_compliance/gst_india/utils/gstr_1/test_gstr_1_books_data.py:791-822
Timestamp: 2025-05-29T15:22:04.761Z
Learning: In the india_compliance test suite, the IntegrationTestCase framework automatically handles database rollbacks after test completion, which means functions that modify global state (like GST Settings and Item Tax Templates) in setUpClass won't persist beyond the test run and don't require manual cleanup.
📚 Learning: 2025-05-29T15:22:04.761Z
Learnt from: vorasmit
Repo: resilient-tech/india-compliance PR: 3399
File: india_compliance/gst_india/utils/gstr_1/test_gstr_1_books_data.py:791-822
Timestamp: 2025-05-29T15:22:04.761Z
Learning: In the india_compliance test suite, the IntegrationTestCase framework automatically handles database rollbacks after test completion, which means functions that modify global state (like GST Settings and Item Tax Templates) in setUpClass won't persist beyond the test run and don't require manual cleanup.
Applied to files:
india_compliance/tests/__init__.py
📚 Learning: 2025-11-21T08:53:26.329Z
Learnt from: ljain112
Repo: resilient-tech/india-compliance PR: 3807
File: india_compliance/gst_india/overrides/transaction.py:142-157
Timestamp: 2025-11-21T08:53:26.329Z
Learning: In the india_compliance/gst_india/overrides/transaction.py file, the `_item_wise_tax_details` field used in `validate_item_wise_tax_detail()` is populated from ERPNext (not from within the India Compliance codebase) before validation runs, as part of cross-repository changes.
Applied to files:
india_compliance/tests/__init__.py
📚 Learning: 2025-11-11T12:44:44.690Z
Learnt from: ljain112
Repo: resilient-tech/india-compliance PR: 3784
File: india_compliance/gst_india/client_scripts/company.js:25-28
Timestamp: 2025-11-11T12:44:44.690Z
Learning: In india_compliance, when using erpnext.company.set_custom_query(frm, [field_name, filters]) in Company doctype client scripts, the function automatically adds company and is_group filters. The filters object parameter should only contain additional filters beyond these defaults (e.g., root_type). For default_gst_expense_account, no root_type filter is needed, so an empty object {} is passed.
Applied to files:
india_compliance/tests/__init__.py
📚 Learning: 2025-09-29T05:06:10.320Z
Learnt from: ljain112
Repo: resilient-tech/india-compliance PR: 3693
File: india_compliance/gst_india/doctype/gst_invoice_management_system/gst_invoice_management_system.js:32-33
Timestamp: 2025-09-29T05:06:10.320Z
Learning: GST-related tool doctypes (like GST Invoice Management System, Purchase Reconciliation Tool, GSTR-1) use unconditional assignment of session default company in their setup() method: `frm.doc.company = frappe.defaults.get_user_default("Company");`. This is appropriate for these utility tools as they are meant to always start with the user's default company on each load, unlike regular documents that preserve saved company values.
Applied to files:
india_compliance/tests/__init__.py
🧬 Code graph analysis (1)
.github/helper/post_install.sh (1)
india_compliance/gst_india/overrides/transaction.py (1)
update(1154-1167)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Python Unit Tests
- GitHub Check: Post Install Patches
- GitHub Check: Mergify Merge Protections
- GitHub Check: Summary
🔇 Additional comments (4)
india_compliance/tests/__init__.py (2)
11-21: LGTM! Clean refactoring of test setup.The extraction of
setup_india_company()improves reusability. The addition offrappe.db.commit()at line 17 ensures that test records and settings are persisted before proceeding, which aligns with the commit pattern insetup_post_install_test_data().
54-56: LGTM! Simple wrapper for post-install testing.The function appropriately calls
setup_india_company()and commits the transaction, making it suitable for use in the CI workflow where a persistent database state is required..github/helper/post_install.sh (2)
38-47: Hardcoded database credentials are acceptable for CI but consider documenting.The hardcoded password
travisand credentialstest_resilientare fine for a CI test environment. Just noting this is intentional test infrastructure.
70-73: LGTM! Clean integration with the new test helper.Using
setup_post_install_test_data()from the refactored test module ensures consistent company setup between CI and local test runs.
| sudo apt update | ||
|
|
||
| sudo apt remove mysql-server mysql-client | ||
| sudo apt install libcups2-dev redis-server mariadb-client |
There was a problem hiding this comment.
Add -y flag to apt commands to prevent interactive prompts in CI.
The apt remove and apt install commands will prompt for confirmation, causing the CI workflow to hang indefinitely. Add the -y flag for non-interactive execution.
Proposed fix
sudo apt update
-sudo apt remove mysql-server mysql-client
-sudo apt install libcups2-dev redis-server mariadb-client
+sudo apt remove -y mysql-server mysql-client
+sudo apt install -y libcups2-dev redis-server mariadb-client📝 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.
| sudo apt update | |
| sudo apt remove mysql-server mysql-client | |
| sudo apt install libcups2-dev redis-server mariadb-client | |
| sudo apt update | |
| sudo apt remove -y mysql-server mysql-client | |
| sudo apt install -y libcups2-dev redis-server mariadb-client |
🤖 Prompt for AI Agents
In @.github/helper/post_install.sh around lines 16 - 19, Update the apt commands
to run non-interactively: modify the commands "sudo apt remove mysql-server
mysql-client" and "sudo apt install libcups2-dev redis-server mariadb-client" to
include the -y flag (e.g., "sudo apt remove -y ..." and "sudo apt install -y
...") so the script won’t prompt during CI runs.
|
|
||
| install_whktml() { | ||
| wget -O /tmp/wkhtmltox.deb https://github.com/wkhtmltopdf/packaging/releases/download/0.12.6.1-2/wkhtmltox_0.12.6.1-2.jammy_amd64.deb | ||
| sudo apt install /tmp/wkhtmltox.deb |
There was a problem hiding this comment.
Add -y flag for non-interactive wkhtmltopdf installation.
The apt install inside the background function also needs the -y flag.
Proposed fix
- sudo apt install /tmp/wkhtmltox.deb
+ sudo apt install -y /tmp/wkhtmltox.deb📝 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.
| sudo apt install /tmp/wkhtmltox.deb | |
| sudo apt install -y /tmp/wkhtmltox.deb |
🤖 Prompt for AI Agents
In @.github/helper/post_install.sh at line 23, The apt install command in the
background installer currently runs interactively ("sudo apt install
/tmp/wkhtmltox.deb"); update that invocation to be non-interactive by adding the
-y flag (change to "sudo apt install -y /tmp/wkhtmltox.deb") and apply the same
-y flag to any other apt install calls inside the same background function so
the script can run unattended.
| def setup_india_company(): | ||
| frappe.clear_cache() | ||
|
|
||
| if not frappe.db.a_row_exists("Company"): | ||
| today = getdate() | ||
| year = today.year if today.month > 3 else today.year - 1 | ||
|
|
||
| setup_args = { | ||
| "currency": "INR", | ||
| "full_name": "Test User", | ||
| "company_name": "Wind Power LLP", | ||
| "timezone": "Asia/Kolkata", | ||
| "company_abbr": "WP", | ||
| "industry": "Manufacturing", | ||
| "country": "India", | ||
| "fy_start_date": f"{year}-04-01", | ||
| "fy_end_date": f"{year + 1}-03-31", | ||
| "language": "English", | ||
| "company_tagline": "Testing", | ||
| "email": "test@example.com", | ||
| "password": "test", | ||
| "chart_of_accounts": "Standard", | ||
| "enable_audit_trail": 0, | ||
| "company_gstin": "29MUMB22923F1D", | ||
| "default_gst_rate": "18.0", | ||
| } | ||
|
|
||
| setup_complete(setup_args) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Count characters in the GSTIN
echo "GSTIN character count:"
echo -n "29MUMB22923F1D" | wc -c
# Find GSTIN_FORMAT regex in the codebase
echo ""
echo "Searching for GSTIN_FORMAT definition:"
rg -n "GSTIN_FORMAT" --type py -B2 -A2
# Find other company_gstin values in tests
echo ""
echo "Searching for other test GSTINs:"
rg -n "company_gstin" --type py -A1 | head -20Repository: resilient-tech/india-compliance
Length of output: 5688
🏁 Script executed:
#!/bin/bash
# Find validate_gstin function implementation
echo "Finding validate_gstin implementation:"
rg -n "def validate_gstin" --type py -A20
# Also check the GSTIN_FORMATS patterns definition
echo ""
echo "Checking GSTIN_FORMATS patterns:"
rg -n "GSTIN_FORMATS\s*=" --type py -A10Repository: resilient-tech/india-compliance
Length of output: 9918
🏁 Script executed:
#!/bin/bash
# Find setup_complete function
echo "Finding setup_complete function:"
rg -n "def setup_complete" --type py -A30
# Also check where setup_complete is imported from in tests/__init__.py
echo ""
echo "Checking imports in tests/__init__.py:"
head -30 india_compliance/tests/__init__.pyRepository: resilient-tech/india-compliance
Length of output: 939
🏁 Script executed:
#!/bin/bash
# Search for setup_wizard in india_compliance
echo "Searching for setup_wizard hooks or overrides in india_compliance:"
rg -n "setup_wizard\|setup_complete" india_compliance --type py -B2 -A2
# Check if validate_gstin is called during company creation
echo ""
echo "Searching for where validate_gstin is called:"
rg -n "validate_gstin\(" --type py | head -20Repository: resilient-tech/india-compliance
Length of output: 989
🏁 Script executed:
#!/bin/bash
# Check the full setup_wizard.py to see how it integrates
echo "Reading india_compliance/setup_wizard.py:"
cat india_compliance/setup_wizard.pyRepository: resilient-tech/india-compliance
Length of output: 3675
Correct the test GSTIN to 15 characters.
The GSTIN 29MUMB22923F1D is 14 characters, but valid Indian GSTINs must be exactly 15 characters (format: 2 state code + 5 PAN letters + 4 PAN digits + 1 entity type + 1 digit/letter + 'Z' + 1 checksum). The validation function in setup_wizard.py will reject this GSTIN and set it to None, defeating the purpose of this test data.
🤖 Prompt for AI Agents
In @india_compliance/tests/__init__.py around lines 24 - 52, The test GSTIN in
setup_india_company is 14 chars and will be rejected; update
setup_args["company_gstin"] to a valid 15-character GSTIN (e.g., follow the
pattern used by the validation: 2-digit state code + 10-char PAN + entity code +
'Z' + checksum) so the value remains set during setup; locate the
setup_india_company function and change the "company_gstin" entry in the
setup_args dict to a correct 15-character GSTIN string.
ljain112
force-pushed
the
test-post-install
branch
from
02d11df to
81eafe8
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/helper/post_install.sh:
- Around line 18-19: Update the two apt commands so they run non-interactively
by adding the -y flag: change the line containing "sudo apt remove mysql-server
mysql-client" to "sudo apt remove -y mysql-server mysql-client" and the line
containing "sudo apt install libcups2-dev redis-server mariadb-client" to "sudo
apt install -y libcups2-dev redis-server mariadb-client"; optionally also set
DEBIAN_FRONTEND=noninteractive in the environment for CI if further
non-interactive behavior is needed.
- Around line 21-24: The function name install_whktml is misspelled and the apt
call is missing the non-interactive flag; rename the function to
install_wkhtmltopdf (or install_wkhtmltox if you prefer that naming) and update
any calls to it, and change the install line to use sudo apt install -y
/tmp/wkhtmltox.deb so the package installs non-interactively; keep the wget
download as-is but ensure any references to install_whktml are updated to the
new function name.
🧹 Nitpick comments (7)
.github/helper/post_install.sh (3)
59-59: Missing error handling for background process.If the wkhtmltopdf installation fails in the background,
wait $wkpidwon't propagate the error due toset -enot applying to background jobs. Consider checking the exit status explicitly.🔧 Proposed fix
-wait $wkpid +wait $wkpid || { echo "wkhtmltopdf installation failed"; exit 1; }
62-63: Backgroundbench startoutput is lost and may cause race condition.Starting bench in background without redirecting output makes debugging difficult. Also,
bench reinstallmay execute before bench is fully started. Consider adding a delay or health check.🔧 Proposed fix
-bench start & -bench reinstall --yes +bench start &> ~/frappe-bench/bench_start.log & +sleep 5 # Allow bench to start +bench reinstall --yesThis also aligns with the workflow's "Show bench output" step (line 106-107 in post-install-tests.yml) which expects a
bench_start.logfile.
70-70: Quote the variable to prevent word splitting.The
${GITHUB_WORKSPACE}variable should be quoted to handle paths with spaces correctly.🔧 Proposed fix
-cd ~/frappe-bench && ./env/bin/python ${GITHUB_WORKSPACE}/.github/helper/setup_post_install_data.py +cd ~/frappe-bench && ./env/bin/python "${GITHUB_WORKSPACE}/.github/helper/setup_post_install_data.py".github/helper/setup_post_install_data.py (2)
1-5: Fix import order and remove leading blank line.The file starts with a blank line, and imports are not in standard order (stdlib before third-party).
🔧 Proposed fix
- -import frappe import os + +import frappe from frappe.utils import get_bench_path + from india_compliance.tests import setup_post_install_test_data
7-16: Missingfrappe.destroy()to clean up database connection.The script initializes and connects to frappe but never cleans up. This can leave database connections open.
🔧 Proposed fix
def main(): # Initialize frappe for the site bench_path = get_bench_path() sites_path = os.path.join(bench_path, "sites") frappe.init(site="test_site", sites_path=sites_path) frappe.connect() - print("Running setup for India Company...") - setup_post_install_test_data() - print("Setup complete.") + try: + print("Running setup for India Company...") + setup_post_install_test_data() + print("Setup complete.") + finally: + frappe.destroy().github/workflows/post-install-tests.yml (2)
89-89: Quote variable to prevent word splitting (shellcheck SC2086).Per static analysis hint, the yarn cache directory path should be quoted.
🔧 Proposed fix
- run: echo "dir=$(yarn cache dir)" >> $GITHUB_OUTPUT + run: echo "dir=$(yarn cache dir)" >> "$GITHUB_OUTPUT"
100-101: Quote variable to prevent word splitting (shellcheck SC2086).Per static analysis hint,
GITHUB_WORKSPACEshould be quoted.🔧 Proposed fix
- bash ${GITHUB_WORKSPACE}/.github/helper/post_install.sh + bash "${GITHUB_WORKSPACE}/.github/helper/post_install.sh"
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/helper/post_install.sh.github/helper/setup_post_install_data.py.github/workflows/post-install-tests.ymlindia_compliance/tests/__init__.py
🚧 Files skipped from review as they are similar to previous changes (1)
- india_compliance/tests/init.py
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: vorasmit
Repo: resilient-tech/india-compliance PR: 3399
File: india_compliance/gst_india/utils/gstr_1/test_gstr_1_books_data.py:791-822
Timestamp: 2025-05-29T15:22:04.761Z
Learning: In the india_compliance test suite, the IntegrationTestCase framework automatically handles database rollbacks after test completion, which means functions that modify global state (like GST Settings and Item Tax Templates) in setUpClass won't persist beyond the test run and don't require manual cleanup.
📚 Learning: 2025-12-26T10:26:32.848Z
Learnt from: karm1000
Repo: resilient-tech/india-compliance PR: 3857
File: .github/workflows/server-tests.yml:48-48
Timestamp: 2025-12-26T10:26:32.848Z
Learning: CI workflows related to server tests should reflect supported environment versions. Since Frappe framework v16 (develop branch) uses MariaDB 11.8, ensure workflows under .github/workflows (e.g., server-tests.yml) test against MariaDB 11.8 by updating the test matrix, Docker compose services, or container images (e.g., mariadb:11.8) and verify compatibility in tests.
Applied to files:
.github/workflows/post-install-tests.yml
🧬 Code graph analysis (2)
.github/helper/post_install.sh (1)
india_compliance/gst_india/overrides/transaction.py (1)
update(1154-1167)
.github/helper/setup_post_install_data.py (1)
india_compliance/tests/__init__.py (1)
setup_post_install_test_data(54-56)
🪛 actionlint (1.7.10)
.github/workflows/post-install-tests.yml
89-89: shellcheck reported issue in this script: SC2086:info:1:33: Double quote to prevent globbing and word splitting
(shellcheck)
100-100: shellcheck reported issue in this script: SC2086:info:1:6: Double quote to prevent globbing and word splitting
(shellcheck)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Mergify Merge Protections
- GitHub Check: Summary
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Python Unit Tests
- GitHub Check: Post Install Patches
🔇 Additional comments (2)
.github/workflows/post-install-tests.yml (2)
39-46: MariaDB 11.8 service configuration looks good.The MariaDB 11.8 image aligns with the supported environment for Frappe framework v16 (develop branch). Based on learnings, this is the correct version to use.
52-55: No action needed. Python 3.14 was released as a stable version on October 7, 2025, and is fully supported byactions/setup-python@v6. The workflow configuration is correct.Likely an incorrect or invalid review comment.
| install_whktml() { | ||
| wget -O /tmp/wkhtmltox.deb https://github.com/wkhtmltopdf/packaging/releases/download/0.12.6.1-2/wkhtmltox_0.12.6.1-2.jammy_amd64.deb | ||
| sudo apt install /tmp/wkhtmltox.deb | ||
| } |
There was a problem hiding this comment.
Typo in function name and missing -y flag.
The function name install_whktml has a typo (should be install_wkhtmltopdf or similar). Additionally, the apt install inside the function needs the -y flag for non-interactive execution.
🔧 Proposed fix
-install_whktml() {
+install_wkhtmltopdf() {
wget -O /tmp/wkhtmltox.deb https://github.com/wkhtmltopdf/packaging/releases/download/0.12.6.1-2/wkhtmltox_0.12.6.1-2.jammy_amd64.deb
- sudo apt install /tmp/wkhtmltox.deb
+ sudo apt install /tmp/wkhtmltox.deb -y
}
-install_whktml &
+install_wkhtmltopdf &🤖 Prompt for AI Agents
In @.github/helper/post_install.sh around lines 21 - 24, The function name
install_whktml is misspelled and the apt call is missing the non-interactive
flag; rename the function to install_wkhtmltopdf (or install_wkhtmltox if you
prefer that naming) and update any calls to it, and change the install line to
use sudo apt install -y /tmp/wkhtmltox.deb so the package installs
non-interactively; keep the wget download as-is but ensure any references to
install_whktml are updated to the new function name.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/helper/post_install.sh:
- Around line 18-19: The sudo apt remove mysql-server mysql-client command can
fail under set -e if those packages are not installed; update that invocation in
.github/helper/post_install.sh (the apt remove line) to tolerate missing
packages by either adding a fallback (e.g., append || true) or switching to an
apt/apt-get option that ignores missing packages (such as --ignore-missing or
using apt-get remove -y with error suppression), ensuring the script does not
exit on a non-zero status when MySQL packages are absent.
- Around line 62-63: The current script launches the server with "bench start &"
then immediately runs "bench reinstall --yes", causing a race; change the flow
to start the server in the background, then wait for the server to become
healthy before running reinstall. Implement a readiness loop after "bench start
&" that polls the local server (e.g., curl or wget against the site root or a
health endpoint such as /api/method/frappe.ping) with retries and a timeout,
proceeding to run "bench reinstall --yes" only once a successful response is
received; abort with a clear error if the timeout is reached.
🧹 Nitpick comments (2)
.github/helper/post_install.sh (2)
21-24: Typo in function name and hardcoded Ubuntu version.
- Function name
install_whktmlappears to be a typo forinstall_wkhtmltopdf- The URL hardcodes
jammy(Ubuntu 22.04) - this may break if the runner uses a different Ubuntu versionProposed fix
-install_whktml() { +install_wkhtmltopdf() { wget -O /tmp/wkhtmltox.deb https://github.com/wkhtmltopdf/packaging/releases/download/0.12.6.1-2/wkhtmltox_0.12.6.1-2.jammy_amd64.deb sudo apt install /tmp/wkhtmltox.deb } -install_whktml & +install_wkhtmltopdf &
62-62: Consider adding cleanup for the background bench process.The
bench start &process runs in the background indefinitely. If the script fails after this point, the process may remain orphaned. Consider adding a trap to ensure cleanup on exit.Proposed improvement
+# Cleanup function +cleanup() { + if [ -n "$BENCH_PID" ]; then + kill $BENCH_PID 2>/dev/null || true + fi +} +trap cleanup EXIT + bench start & +BENCH_PID=$!
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/helper/post_install.sh
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: vorasmit
Repo: resilient-tech/india-compliance PR: 3399
File: india_compliance/gst_india/utils/gstr_1/test_gstr_1_books_data.py:791-822
Timestamp: 2025-05-29T15:22:04.761Z
Learning: In the india_compliance test suite, the IntegrationTestCase framework automatically handles database rollbacks after test completion, which means functions that modify global state (like GST Settings and Item Tax Templates) in setUpClass won't persist beyond the test run and don't require manual cleanup.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Python Unit Tests
- GitHub Check: Post Install Patches
- GitHub Check: Mergify Merge Protections
- GitHub Check: Summary
🔇 Additional comments (4)
.github/helper/post_install.sh (4)
1-10: LGTM!The preflight checks for merge conflicts and Python syntax errors are appropriate safeguards for CI.
28-47: LGTM!The bench initialization, site directory setup, and MariaDB provisioning are appropriate for a CI test environment.
65-81: LGTM!The deliberate ordering—getting the app after reinstall, creating India company data before installation—is the correct approach for testing post-install patches. The Python inline execution with
sys.pathmanipulation is appropriate since the app isn't formally installed yet.
83-89: LGTM!The installation and finalization steps are appropriate.
| sudo apt remove mysql-server mysql-client | ||
| sudo apt install libcups2-dev redis-server mariadb-client |
There was a problem hiding this comment.
apt remove may fail if MySQL packages aren't installed.
With set -e enabled, if mysql-server or mysql-client aren't installed, this command could exit with a non-zero status and terminate the script. Consider using || true to ignore errors, or the --ignore-missing flag (available in newer apt versions).
Proposed fix
-sudo apt remove mysql-server mysql-client
+sudo apt remove mysql-server mysql-client || true📝 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.
| sudo apt remove mysql-server mysql-client | |
| sudo apt install libcups2-dev redis-server mariadb-client | |
| sudo apt remove mysql-server mysql-client || true | |
| sudo apt install libcups2-dev redis-server mariadb-client |
🤖 Prompt for AI Agents
In @.github/helper/post_install.sh around lines 18 - 19, The sudo apt remove
mysql-server mysql-client command can fail under set -e if those packages are
not installed; update that invocation in .github/helper/post_install.sh (the apt
remove line) to tolerate missing packages by either adding a fallback (e.g.,
append || true) or switching to an apt/apt-get option that ignores missing
packages (such as --ignore-missing or using apt-get remove -y with error
suppression), ensuring the script does not exit on a non-zero status when MySQL
packages are absent.
| bench start & | ||
| bench reinstall --yes |
There was a problem hiding this comment.
Race condition: bench reinstall may run before server is ready.
bench start & spawns the server in the background, but bench reinstall --yes executes immediately without waiting for the server to be fully operational. This can cause intermittent failures if the server isn't ready to handle requests.
Proposed fix - add a readiness check
bench start &
+
+# Wait for the server to be ready
+timeout 60 bash -c 'until curl -s http://localhost:8000 > /dev/null 2>&1; do sleep 2; done'
+
bench reinstall --yes📝 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.
| bench start & | |
| bench reinstall --yes | |
| bench start & | |
| # Wait for the server to be ready | |
| timeout 60 bash -c 'until curl -s http://localhost:8000 > /dev/null 2>&1; do sleep 2; done' | |
| bench reinstall --yes |
🤖 Prompt for AI Agents
In @.github/helper/post_install.sh around lines 62 - 63, The current script
launches the server with "bench start &" then immediately runs "bench reinstall
--yes", causing a race; change the flow to start the server in the background,
then wait for the server to become healthy before running reinstall. Implement a
readiness loop after "bench start &" that polls the local server (e.g., curl or
wget against the site root or a health endpoint such as /api/method/frappe.ping)
with retries and a timeout, proceeding to run "bench reinstall --yes" only once
a successful response is received; abort with a clear error if the timeout is
reached.
1 participant
Summary by CodeRabbit
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.