Skip to content

fix: update default base branch reference in server tests workflow#4023

Draft
Abdeali099 wants to merge 1 commit intodevelopfrom
fix-base-branch-dev
Draft

fix: update default base branch reference in server tests workflow#4023
Abdeali099 wants to merge 1 commit intodevelopfrom
fix-base-branch-dev

Conversation

@Abdeali099
Copy link
Copy Markdown
Member

@Abdeali099 Abdeali099 marked this pull request as draft February 25, 2026 06:36
@Abdeali099
Copy link
Copy Markdown
Member Author

need to discuss with @sagarvora

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Feb 25, 2026

📝 Walkthrough

Walkthrough

The workflow configuration file .github/workflows/server-tests.yml was updated to modify the default branch resolution behavior. Previously, the BRANCH environment variable resolved dynamically using GitHub context values (github.base_ref, github.ref_name) and input parameters. The change removes this dynamic fallback logic and instead defaults to the literal string 'develop' when no explicit input is provided.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: updating the default base branch reference in the server tests workflow.
Description check ✅ Passed The description mentions issue #4022, which directly relates to the changeset addressing hard-coded base branch references.
Linked Issues check ✅ Passed The PR updates the default base branch from dynamic GitHub-provided refs to 'develop', directly addressing issue #4022's objective to adjust base branch handling in workflows.
Out of Scope Changes check ✅ Passed The change is narrowly scoped to updating the default BRANCH variable in the workflow file, which is directly related to the linked issue objective.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-base-branch-dev

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.

❤️ Share

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

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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/server-tests.yml:
- Line 33: Restore branch-aware fallback for BRANCH by replacing the current
default with a fallback that uses inputs.base_ref first, then the event/runner
branch values: use inputs.base_ref || github.head_ref || github.ref_name ||
'develop' (referencing the BRANCH assignment and symbols inputs.base_ref,
github.head_ref, github.ref_name) so pushes and pull_request runs pick up the
actual branch name instead of always defaulting to develop.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 653aa84 and 41acdc6.

📒 Files selected for processing (1)
  • .github/workflows/server-tests.yml

- "**.csv"
env:
BRANCH: ${{ inputs.base_ref || github.base_ref || github.ref_name }}
BRANCH: ${{ inputs.base_ref || 'develop' }}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

❓ Verification inconclusive

Script executed:

cat -n .github/workflows/server-tests.yml | head -80

Repository: resilient-tech/india-compliance


Repository: resilient-tech/india-compliance
Exit code: 0

stdout:

     1	name: Server Tests
     2	
     3	on:
     4	    workflow_call:
     5	      inputs:
     6	        base_ref:
     7	          type : string
     8	
     9	        app_name:
    10	          type : string
    11	
    12	      secrets:
    13	        codecov_token:
    14	
    15	
    16	    pull_request:
    17	        paths-ignore:
    18	            - "**.css"
    19	            - "**.js"
    20	            - "**.md"
    21	            - "**.html"
    22	            - "**.csv"
    23	
    24	    push:
    25	        branches: [develop, version-14-hotfix, version-14, version-15-hotfix, version-15, version-16-hotfix, version-16]
    26	        paths-ignore:
    27	            - "**.css"
    28	            - "**.js"
    29	            - "**.md"
    30	            - "**.html"
    31	            - "**.csv"
    32	env:
    33	    BRANCH: ${{ inputs.base_ref || 'develop' }}
    34	    APP_NAME: ${{ inputs.app_name || 'india_compliance' }}
    35	
    36	jobs:
    37	    tests:
    38	        runs-on: ubuntu-latest
    39	        timeout-minutes: 20
    40	
    41	        strategy:
    42	            fail-fast: false
    43	
    44	        name: Python Unit Tests
    45	
    46	        services:
    47	            mariadb:
    48	                image: mariadb:11.8
    49	                env:
    50	                    MARIADB_ROOT_PASSWORD: 'travis'
    51	                ports:
    52	                    - 3306:3306
    53	                options: --health-cmd="mariadb-admin ping" --health-interval=5s --health-timeout=2s --health-retries=3
    54	
    55	        steps:
    56	            - name: Clone
    57	              uses: actions/checkout@v6
    58	
    59	            - name: Setup Python
    60	              uses: actions/setup-python@v6
    61	              with:
    62	                  python-version: "3.14"
    63	
    64	            - name: Setup Node
    65	              uses: actions/setup-node@v6
    66	              with:
    67	                  node-version: 24
    68	                  check-latest: true
    69	
    70	            - name: Add to Hosts
    71	              run: echo "127.0.0.1 test_site" | sudo tee -a /etc/hosts
    72	
    73	            - name: Cache pip
    74	              uses: actions/cache@v5
    75	              with:
    76	                  path: ~/.cache/pip
    77	                  key: ${{ runner.os }}-pip-${{ hashFiles('**/*requirements.txt') }}
    78	                  restore-keys: |
    79	                      ${{ runner.os }}-pip-
    80	                      ${{ runner.os }}-

Script executed:

# Check the full context around line 33 and understand the workflow triggers
cat -n .github/workflows/server-tests.yml | sed -n '1,60p'

Repository: resilient-tech/india-compliance


Repository: resilient-tech/india-compliance
Exit code: 0

stdout:

     1	name: Server Tests
     2	
     3	on:
     4	    workflow_call:
     5	      inputs:
     6	        base_ref:
     7	          type : string
     8	
     9	        app_name:
    10	          type : string
    11	
    12	      secrets:
    13	        codecov_token:
    14	
    15	
    16	    pull_request:
    17	        paths-ignore:
    18	            - "**.css"
    19	            - "**.js"
    20	            - "**.md"
    21	            - "**.html"
    22	            - "**.csv"
    23	
    24	    push:
    25	        branches: [develop, version-14-hotfix, version-14, version-15-hotfix, version-15, version-16-hotfix, version-16]
    26	        paths-ignore:
    27	            - "**.css"
    28	            - "**.js"
    29	            - "**.md"
    30	            - "**.html"
    31	            - "**.csv"
    32	env:
    33	    BRANCH: ${{ inputs.base_ref || 'develop' }}
    34	    APP_NAME: ${{ inputs.app_name || 'india_compliance' }}
    35	
    36	jobs:
    37	    tests:
    38	        runs-on: ubuntu-latest
    39	        timeout-minutes: 20
    40	
    41	        strategy:
    42	            fail-fast: false
    43	
    44	        name: Python Unit Tests
    45	
    46	        services:
    47	            mariadb:
    48	                image: mariadb:11.8
    49	                env:
    50	                    MARIADB_ROOT_PASSWORD: 'travis'
    51	                ports:
    52	                    - 3306:3306
    53	                options: --health-cmd="mariadb-admin ping" --health-interval=5s --health-timeout=2s --health-retries=3
    54	
    55	        steps:
    56	            - name: Clone
    57	              uses: actions/checkout@v6
    58	
    59	            - name: Setup Python
    60	              uses: actions/setup-python@v6

Script executed:

# Find all usages of BRANCH variable in the workflow
rg -n 'BRANCH' .github/workflows/server-tests.yml

Repository: resilient-tech/india-compliance


Repository: resilient-tech/india-compliance
Exit code: 0

stdout:

33:    BRANCH: ${{ inputs.base_ref || 'develop' }}
110:                  BRANCH_TO_CLONE: ${{ env.BRANCH }}

Script executed:

# Check if install.sh exists and how it uses BRANCH_TO_CLONE
ls -la .github/helper/ 2>/dev/null || echo "Helper directory not found"

Repository: resilient-tech/india-compliance


Repository: resilient-tech/india-compliance
Exit code: 0

stdout:

total 12
drwxr-xr-x 2 jailuser jailuser  100 Feb 25 06:34 .
drwxr-xr-x 5 jailuser jailuser  120 Feb 25 06:34 ..
-rw-r--r-- 1 jailuser jailuser 1922 Feb 25 06:34 documentation.py
-rw-r--r-- 1 jailuser jailuser 1843 Feb 25 06:34 install.sh
-rw-r--r-- 1 jailuser jailuser  465 Feb 25 06:34 site_config.json

Script executed:

cat -n .github/helper/install.sh

Repository: resilient-tech/india-compliance


Repository: resilient-tech/india-compliance
Exit code: 0

stdout:

     1	#!/bin/bash
     2	
     3	set -e
     4	
     5	# Check for merge conflicts before proceeding
     6	python -m compileall -f "${GITHUB_WORKSPACE}"
     7	if grep -lr --exclude-dir=node_modules "^<<<<<<< " "${GITHUB_WORKSPACE}"
     8	    then echo "Found merge conflicts"
     9	    exit 1
    10	fi
    11	
    12	cd ~ || exit
    13	
    14	echo "Setting Up System Dependencies..."
    15	
    16	sudo apt update
    17	
    18	sudo apt remove mysql-server mysql-client
    19	sudo apt install libcups2-dev redis-server mariadb-client
    20	
    21	install_whktml() {
    22	    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
    23	    sudo apt install /tmp/wkhtmltox.deb
    24	}
    25	install_whktml &
    26	wkpid=$!
    27	
    28	pip install frappe-bench
    29	
    30	git clone "https://github.com/frappe/frappe" --branch "$BRANCH_TO_CLONE" --depth 1
    31	bench init --skip-assets --frappe-path ~/frappe --python "$(which python)" frappe-bench
    32	
    33	mkdir ~/frappe-bench/sites/test_site
    34	
    35	cp -r "${GITHUB_WORKSPACE}/.github/helper/site_config.json" ~/frappe-bench/sites/test_site/
    36	
    37	
    38	mariadb --host 127.0.0.1 --port 3306 -u root -ptravis -e "
    39	SET GLOBAL character_set_server = 'utf8mb4';
    40	SET GLOBAL collation_server = 'utf8mb4_unicode_ci';
    41	
    42	CREATE USER 'test_resilient'@'localhost' IDENTIFIED BY 'test_resilient';
    43	CREATE DATABASE test_resilient;
    44	GRANT ALL PRIVILEGES ON \`test_resilient\`.* TO 'test_resilient'@'localhost';
    45	
    46	FLUSH PRIVILEGES;
    47	"
    48	
    49	cd ~/frappe-bench || exit
    50	
    51	sed -i 's/watch:/# watch:/g' Procfile
    52	sed -i 's/schedule:/# schedule:/g' Procfile
    53	sed -i 's/socketio:/# socketio:/g' Procfile
    54	sed -i 's/redis_socketio:/# redis_socketio:/g' Procfile
    55	
    56	bench get-app erpnext --branch "$BRANCH_TO_CLONE" --resolve-deps
    57	bench get-app india_compliance "${GITHUB_WORKSPACE}"
    58	bench setup requirements --dev
    59	
    60	wait $wkpid
    61	
    62	bench use test_site
    63	bench start &
    64	bench reinstall --yes
    65	
    66	bench --verbose install-app india_compliance
    67	bench --site test_site add-to-hosts
    68	

Script executed:

# Check the full workflow to understand context of line 110 where BRANCH_TO_CLONE is used
sed -n '100,130p' .github/workflows/server-tests.yml

Repository: resilient-tech/india-compliance


Repository: resilient-tech/india-compliance
Exit code: 0

stdout:

              with:
                  path: ${{ steps.yarn-cache-dir-path.outputs.dir }}
                  key: ${{ runner.os }}-yarn-${{ hashFiles('**/yarn.lock') }}
                  restore-keys: |
                      ${{ runner.os }}-yarn-

            - name: Install
              run: |
                  bash ${GITHUB_WORKSPACE}/.github/helper/install.sh
              env:
                  BRANCH_TO_CLONE: ${{ env.BRANCH }}

            - name: Run Tests
              run: cd ~/frappe-bench/ && bench --site test_site run-parallel-tests --app ${{ env.APP_NAME }} --with-coverage
              env:
                  TYPE: server

            - name: Show bench output
              if: ${{ always() }}
              run: cat ~/frappe-bench/bench_start.log || true

            - name: Upload coverage data
              uses: actions/upload-artifact@v6
              with:
                name: coverage
                path: /home/runner/frappe-bench/sites/coverage.xml

    coverage:
        name: Coverage Wrap Up
        if: github.event_name == 'push'
        needs: tests

Script executed:

# Verify the semantics of github.base_ref and github.ref_name context availability
# Check if there's any documentation or comments in the workflow about these variables
rg -i 'github\.(base_ref|ref_name|event)' .github/workflows/server-tests.yml -C 2

Repository: resilient-tech/india-compliance


Repository: resilient-tech/india-compliance
Exit code: 0

stdout:

    coverage:
        name: Coverage Wrap Up
        if: github.event_name == 'push'
        needs: tests
        runs-on: ubuntu-latest

Restore branch-aware fallback for BRANCH.

Line 33 now defaults to develop for push and pull_request events since inputs.base_ref is only available during workflow_call. This breaks multi-branch testing: pushes to version-14, version-15, version-16 branches clone Frappe and ERPNext from develop instead of their respective maintenance branches, causing tests to validate against incorrect dependencies.

🔧 Proposed fix
-    BRANCH: ${{ inputs.base_ref || 'develop' }}
+    BRANCH: ${{ inputs.base_ref || github.base_ref || github.ref_name || 'develop' }}
📝 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
BRANCH: ${{ inputs.base_ref || 'develop' }}
BRANCH: ${{ inputs.base_ref || github.base_ref || github.ref_name || 'develop' }}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/server-tests.yml at line 33, Restore branch-aware fallback
for BRANCH by replacing the current default with a fallback that uses
inputs.base_ref first, then the event/runner branch values: use inputs.base_ref
|| github.head_ref || github.ref_name || 'develop' (referencing the BRANCH
assignment and symbols inputs.base_ref, github.head_ref, github.ref_name) so
pushes and pull_request runs pick up the actual branch name instead of always
defaulting to develop.

- "**.csv"
env:
BRANCH: ${{ inputs.base_ref || github.base_ref || github.ref_name }}
BRANCH: ${{ inputs.base_ref || 'develop' }}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
BRANCH: ${{ inputs.base_ref || 'develop' }}
BRANCH: ${{ github.event.inputs.branch || github.base_ref || 'develop' }}

Maybe this?

Copy link
Copy Markdown
Member Author

@Abdeali099 Abdeali099 Mar 3, 2026

Choose a reason for hiding this comment

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

github.base_ref is problematic,

It will take base branch as input example A, but A not exist in frappe.

and github.event.inputs.branch is not useful because we are not defining workflow_dispatch in on section for manual trigger.

But I think we can use it by changing main section as ERPNext.

https://github.com/frappe/erpnext/blob/40a5b14d642f31a6d2302b871b9e3f9434741ef5/.github/workflows/server-tests-mariadb.yml#L19

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.

DX: Hardcore base branch of frappe and erpnext in test

2 participants