Merge pull request #27957 from blaggacao/ci/add-trick

ci: use dynamic total job

.github/helper/ci.py

@@ -1,9 +1,29 @@
+"""
+Code Coverage and Parallel Test Runner Script
+
+This script is designed to run parallel tests for Frappe applications with optional code coverage.
+It sets up the test environment, handles code coverage configuration, and executes tests using
+either a local parallel test runner or an orchestrator-based runner.
+
+Key features:
+- Configurable code coverage for specific apps
+- Support for local parallel testing and orchestrator-based testing
+- Customizable inclusion and exclusion patterns for coverage
+- Environment variable based configuration
+
+Usage:
+This script is typically run as part of a CI/CD pipeline or for local development testing.
+It can be configured using environment variables such as SITE, ORCHESTRATOR_URL, WITH_COVERAGE, etc.
+"""
+
 # Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and Contributors
 # MIT License. See LICENSE
+
 import json
 import os
 from pathlib import Path
 
+# Define standard patterns for file inclusions and exclusions in coverage
 STANDARD_INCLUSIONS = ["*.py"]
 
 STANDARD_EXCLUSIONS = [
@@ -22,7 +42,7 @@ STANDARD_EXCLUSIONS = [
 	".github/*",
 ]
 
-# tested via commands' test suite
+# Files that are tested via command line interface
 TESTED_VIA_CLI = [
 	"*/frappe/installer.py",
 	"*/frappe/utils/install.py",
@@ -34,14 +54,36 @@ TESTED_VIA_CLI = [
 	"*/frappe/database/**/setup_db.py",
 ]
 
-FRAPPE_EXCLUSIONS = ["*/tests/*", "*/commands/*", "*/frappe/change_log/*", "*/frappe/exceptions*", "*/frappe/desk/page/setup_wizard/setup_wizard.py", "*/frappe/coverage.py", "*frappe/setup.py", "*/frappe/hooks.py", "*/doctype/*/*_dashboard.py", "*/patches/*", "*/.github/helper/ci.py", *TESTED_VIA_CLI]
+# Additional exclusions specific to the Frappe app
+FRAPPE_EXCLUSIONS = [
+	"*/tests/*",
+	"*/commands/*",
+	"*/frappe/change_log/*",
+	"*/frappe/exceptions*",
+	"*/frappe/desk/page/setup_wizard/setup_wizard.py",
+	"*/frappe/coverage.py",
+	"*frappe/setup.py",
+	"*/frappe/hooks.py",
+	"*/doctype/*/*_dashboard.py",
+	"*/patches/*",
+	"*/.github/helper/ci.py",
+	*TESTED_VIA_CLI,
+]
 
 
 def get_bench_path():
+	"""Get the path to the bench directory."""
 	return Path(__file__).resolve().parents[4]
 
 
 class CodeCoverage:
+	"""
+	Context manager for handling code coverage.
+
+	This class sets up code coverage measurement for a specific app,
+	applying the appropriate inclusion and exclusion patterns.
+	"""
+
 	def __init__(self, with_coverage, app):
 		self.with_coverage = with_coverage
 		self.app = app or "frappe"
@@ -49,10 +91,9 @@ class CodeCoverage:
 	def __enter__(self):
 		if self.with_coverage:
 			import os
-
 			from coverage import Coverage
 
-			# Generate coverage report only for app that is being tested
+			# Set up coverage for the specific app
 			source_path = os.path.join(get_bench_path(), "apps", self.app)
 			print(f"Source path: {source_path}")
 			omit = STANDARD_EXCLUSIONS[:]
@@ -71,6 +112,7 @@ class CodeCoverage:
 
 
 if __name__ == "__main__":
+	# Configuration
 	app = "frappe"
 	site = os.environ.get("SITE") or "test_site"
 	use_orchestrator = bool(os.environ.get("ORCHESTRATOR_URL"))
@@ -78,6 +120,7 @@ if __name__ == "__main__":
 	build_number = 1
 	total_builds = 1
 
+	# Parse build information from environment variables
 	try:
 		build_number = int(os.environ.get("BUILD_NUMBER"))
 	except Exception:
@@ -88,12 +131,30 @@ if __name__ == "__main__":
 	except Exception:
 		pass
 
+	# Run tests with code coverage
 	with CodeCoverage(with_coverage=with_coverage, app=app):
+		# Add ASCII banner at the end
 		if use_orchestrator:
 			from frappe.parallel_test_runner import ParallelTestWithOrchestrator
 
-			ParallelTestWithOrchestrator(app, site=site)
+			runner = ParallelTestWithOrchestrator(app, site=site)
 		else:
 			from frappe.parallel_test_runner import ParallelTestRunner
 
-			ParallelTestRunner(app, site=site, build_number=build_number, total_builds=total_builds)
+			runner = ParallelTestRunner(app, site=site, build_number=build_number, total_builds=total_builds)
+
+		mode = "Orchestrator" if use_orchestrator else "Parallel"
+		banner = f"""
+		╔════════════════════════════════════════════╗
+		║     CI Helper Script Execution Summary     ║
+		╠════════════════════════════════════════════╣
+		║ Mode:           {mode:<26} ║
+		║ App:            {app:<26} ║
+		║ Site:           {site:<26} ║
+		║ Build Number:   {build_number:<26} ║
+		║ Total Builds:   {total_builds:<26} ║
+		║ Tests in Build: ~{runner.total_tests:<25} ║
+		╚════════════════════════════════════════════╝
+		"""
+		print(banner)
+		runner.setup_and_run()

.github/helper/install_bench.sh

@@ -2,17 +2,34 @@
 set -e
 cd ~ || exit
 
+verbosity="${BENCH_VERBOSITY_FLAG:-}"
+
+start_time=$(date +%s)
 echo "::group::Install Bench"
 pip install frappe-bench
 echo "::endgroup::"
+end_time=$(date +%s)
+echo "Time taken to Install Bench: $((end_time - start_time)) seconds"
 
-echo "::group::Init Bench"
-bench -v init frappe-bench --skip-assets --python "$(which python)" --frappe-path "${GITHUB_WORKSPACE}"
-cd ./frappe-bench || exit
+git config --global init.defaultBranch main
+git config --global advice.detachedHead false
 
-bench -v setup requirements --dev
+start_time=$(date +%s)
+echo "::group::Init Bench & Install Frappe"
+bench $verbosity init frappe-bench --skip-assets --python "$(which python)" --frappe-path "${GITHUB_WORKSPACE}"
+echo "::endgroup::"
+end_time=$(date +%s)
+echo "Time taken to Init Bench & Install Frappe: $((end_time - start_time)) seconds"
+
+cd ~/frappe-bench || exit
+
+start_time=$(date +%s)
+echo "::group::Install App Requirements"
+bench $verbosity setup requirements --dev
 if [ "$TYPE" == "ui" ]
 then
-  bench -v setup requirements --node;
+  bench $verbosity setup requirements --node;
 fi
+end_time=$(date +%s)
 echo "::endgroup::"
+echo "Time taken to Install App Requirements: $((end_time - start_time)) seconds"

.github/helper/roulette.py

@@ -1,3 +1,23 @@
+"""
+GitHub Pull Request Analysis and CI Build Decision Script
+
+This script analyzes changes in a GitHub pull request and determines whether to run specific CI builds.
+It checks for file types changed, presence of certain labels, and other criteria to make decisions about
+which parts of the CI pipeline should run. The script is designed to optimize CI resources by skipping
+unnecessary builds based on the nature of changes in the pull request.
+
+Key features:
+- Fetches pull request data from GitHub API
+- Analyzes changed files
+- Checks for specific labels on the pull request
+- Determines whether to run server-side, UI, or all tests
+- Handles rate limiting for GitHub API requests
+- Supports environment variables for configuration
+
+Usage:
+This script is intended to be run as part of a CI pipeline, with the necessary environment variables set.
+"""
+
 import json
 import os
 import re
@@ -12,17 +32,28 @@ from urllib.error import HTTPError
 
 @cache
 def fetch_pr_data(pr_number, repo, endpoint=""):
-	api_url = f"https://api.github.com/repos/{repo}/pulls/{pr_number}"
+	"""
+	Fetch pull request data from GitHub API.
 
+	:param pr_number: Pull request number
+	:param repo: Repository name (e.g., "frappe/frappe")
+	:param endpoint: Additional API endpoint (e.g., "files")
+	:return: JSON response from GitHub API
+	"""
+	api_url = f"https://api.github.com/repos/{repo}/pulls/{pr_number}"
 	if endpoint:
 		api_url += f"/{endpoint}"
-
 	res = req(api_url)
 	return json.loads(res.read().decode("utf8"))
 
 
 def req(url):
-	"Simple resilient request call to handle rate limits."
+	"""
+	Make a resilient request to handle rate limits.
+
+	:param url: URL to request
+	:return: URLResponse object
+	"""
 	headers = None
 	token = os.environ.get("GITHUB_TOKEN")
 	if token:
@@ -42,28 +73,53 @@ def req(url):
 
 
 def get_files_list(pr_number, repo="frappe/frappe"):
+	"""
+	Get list of files changed in the pull request.
+
+	:param pr_number: Pull request number
+	:param repo: Repository name
+	:return: List of changed file names
+	"""
 	return [change["filename"] for change in fetch_pr_data(pr_number, repo, "files")]
 
 
 def get_output(command, shell=True):
+	"""
+	Execute a shell command and return its output.
+
+	:param command: Command to execute
+	:param shell: Whether to use shell
+	:return: Command output as string
+	"""
 	print(command)
 	command = shlex.split(command)
 	return subprocess.check_output(command, shell=shell, encoding="utf8").strip()
 
 
 def has_skip_ci_label(pr_number, repo="frappe/frappe"):
+	"""Check if the PR has the 'Skip CI' label."""
 	return has_label(pr_number, "Skip CI", repo)
 
 
 def has_run_server_tests_label(pr_number, repo="frappe/frappe"):
+	"""Check if the PR has the 'Run Server Tests' label."""
 	return has_label(pr_number, "Run Server Tests", repo)
 
 
 def has_run_ui_tests_label(pr_number, repo="frappe/frappe"):
+	"""Check if the PR has the 'Run UI Tests' label."""
 	return has_label(pr_number, "Run UI Tests", repo)
 
 
 def has_label(pr_number, label, repo="frappe/frappe"):
+	"""
+	Check if the pull request has a specific label.
+
+	:param pr_number: Pull request number
+	:param label: Label to check for
+	:param repo: Repository name
+	:return: Boolean indicating presence of label
+	"""
 	return any(
 		[
 			fetched_label["name"]
@@ -74,19 +130,22 @@ def has_label(pr_number, label, repo="frappe/frappe"):
 
 
 def is_server_side_code(file):
-	"""File exclusively affects server side code"""
+	"""Check if the file is server-side code (Python or .po files)."""
 	return file.endswith("py") or file.endswith(".po")
 
 
 def is_ci(file):
+	"""Check if the file is related to CI configuration."""
 	return ".github" in file
 
 
 def is_frontend_code(file):
+	"""Check if the file is frontend code."""
 	return file.lower().endswith((".css", ".scss", ".less", ".sass", ".styl", ".js", ".ts", ".vue", ".html"))
 
 
 def is_docs(file):
+	"""Check if the file is documentation or image."""
 	regex = re.compile(r"\.(md|png|jpg|jpeg|csv|svg)$|^.github|LICENSE")
 	return bool(regex.search(file))
 
@@ -97,23 +156,26 @@ if __name__ == "__main__":
 	pr_number = os.environ.get("PR_NUMBER")
 	repo = os.environ.get("REPO_NAME")
 
-	# this is a push build, run all builds
+	# If it's a push build, run all builds
 	if not pr_number:
 		os.system('echo "build=strawberry" >> $GITHUB_OUTPUT')
 		sys.exit(0)
 
+	# Get list of changed files if not provided
 	files_list = files_list or get_files_list(pr_number=pr_number, repo=repo)
 
 	if not files_list:
 		print("No files' changes detected. Build is shutting")
 		sys.exit(0)
 
+	# Analyze changed files
 	ci_files_changed = any(f for f in files_list if is_ci(f))
 	only_docs_changed = len(list(filter(is_docs, files_list))) == len(files_list)
 	only_frontend_code_changed = len(list(filter(is_frontend_code, files_list))) == len(files_list)
 	updated_py_file_count = len(list(filter(is_server_side_code, files_list)))
 	only_py_changed = updated_py_file_count == len(files_list)
 
+	# Check for Skip CI label and other conditions
 	if has_skip_ci_label(pr_number, repo):
 		if build_type == "ui" and has_run_ui_tests_label(pr_number, repo):
 			print("Running UI tests only.")
@@ -122,14 +184,11 @@ if __name__ == "__main__":
 		else:
 			print("Found `Skip CI` label on pr, stopping build process.")
 			sys.exit(0)
-
 	elif ci_files_changed:
 		print("CI related files were updated, running all build processes.")
-
 	elif only_docs_changed:
 		print("Only docs were updated, stopping build process.")
 		sys.exit(0)
-
 	elif (
 		only_frontend_code_changed
 		and build_type == "server"
@@ -137,9 +196,9 @@ if __name__ == "__main__":
 	):
 		print("Only Frontend code was updated; Stopping Python build process.")
 		sys.exit(0)
-
 	elif build_type == "ui" and only_py_changed and not has_run_ui_tests_label(pr_number, repo):
 		print("Only Python code was updated, stopping Cypress build process.")
 		sys.exit(0)
 
+	# If we reach here, run the build
 	os.system('echo "build=strawberry" >> $GITHUB_OUTPUT')

.github/workflows/patch-mariadb-tests.yml

@@ -99,21 +99,13 @@ jobs:
       - name: Install Dependencies
         run: |
           bash ${GITHUB_WORKSPACE}/.github/helper/install_dependencies.sh
-        env:
-          BEFORE: ${{ env.GITHUB_EVENT_PATH.before }}
-          AFTER: ${{ env.GITHUB_EVENT_PATH.after }}
-          TYPE: server
-          DB: mariadb
 
       - name: Init Bench
         run: |
           pip install frappe-bench
           bash ${GITHUB_WORKSPACE}/.github/helper/install_bench.sh
         env:
-          BEFORE: ${{ env.GITHUB_EVENT_PATH.before }}
-          AFTER: ${{ env.GITHUB_EVENT_PATH.after }}
           TYPE: server
-          DB: mariadb
 
       - name: Init Test Site
         run: |

.github/workflows/server-tests.yml

@@ -36,6 +36,7 @@ jobs:
           PR_NUMBER: ${{ github.event.number }}
           REPO_NAME: ${{ github.repository }}
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+     
 
   test:
     name: Unit Tests
@@ -80,6 +81,12 @@ jobs:
           - 3000:80
 
     steps:
+      - name: Has pyproject.toml changed?
+        id: changed-pyproject
+        uses: tj-actions/changed-files@v45
+        with:
+          files: pyproject.toml
+
       - name: Clone
         uses: actions/checkout@v4
 
@@ -96,6 +103,15 @@ jobs:
               exit 1
           fi
 
+      - name: Check if pyproject.toml changed
+        id: check_changes
+        run: |
+          if git diff --name-only origin/${{ github.base_ref }}..HEAD | grep -q "pyproject.toml"; then
+            echo "pyproject_changed=true" >> $GITHUB_OUTPUT
+          else
+            echo "pyproject_changed=false" >> $GITHUB_OUTPUT
+          fi
+
       - uses: actions/setup-node@v4
         with:
           node-version: 18
@@ -129,22 +145,13 @@ jobs:
       - name: Install Dependencies
         run: |
           bash ${GITHUB_WORKSPACE}/.github/helper/install_dependencies.sh
-        env:
-          BEFORE: ${{ env.GITHUB_EVENT_PATH.before }}
-          AFTER: ${{ env.GITHUB_EVENT_PATH.after }}
-          FRAPPE_SENTRY_DSN: ${{ secrets.SENTRY_DSN }}
-          TYPE: server
-          DB: ${{ matrix.db }}
 
       - name: Init Bench
         run: |
           bash ${GITHUB_WORKSPACE}/.github/helper/install_bench.sh
         env:
-          BEFORE: ${{ env.GITHUB_EVENT_PATH.before }}
-          AFTER: ${{ env.GITHUB_EVENT_PATH.after }}
-          FRAPPE_SENTRY_DSN: ${{ secrets.SENTRY_DSN }}
           TYPE: server
-          DB: ${{ matrix.db }}
+          BENCH_VERBOSITY_FLAG: ${{ steps.changed-pyproject.outputs.any_modified  == 'true' && '-v' || ''}}
 
       - name: Init Test Site
         run: |

.github/workflows/ui-tests.yml

@@ -118,22 +118,12 @@ jobs:
       - name: Install Dependencies
         run: |
           bash ${GITHUB_WORKSPACE}/.github/helper/install_dependencies.sh
-        env:
-          BEFORE: ${{ env.GITHUB_EVENT_PATH.before }}
-          AFTER: ${{ env.GITHUB_EVENT_PATH.after }}
-          FRAPPE_SENTRY_DSN: ${{ secrets.SENTRY_DSN }}
-          TYPE: ui
-          DB: mariadb
 
       - name: Init Bench
         run: |
           bash ${GITHUB_WORKSPACE}/.github/helper/install_bench.sh
         env:
-          BEFORE: ${{ env.GITHUB_EVENT_PATH.before }}
-          AFTER: ${{ env.GITHUB_EVENT_PATH.after }}
-          FRAPPE_SENTRY_DSN: ${{ secrets.SENTRY_DSN }}
           TYPE: ui
-          DB: mariadb
 
       - name: Init Test Site
         run: |

frappe/commands/utils.py

@@ -868,13 +868,14 @@ def run_parallel_tests(
 		else:
 			from frappe.parallel_test_runner import ParallelTestRunner
 
-			ParallelTestRunner(
+			runner = ParallelTestRunner(
 				app,
 				site=site,
 				build_number=build_number,
 				total_builds=total_builds,
 				dry_run=dry_run,
 			)
+			runner.setup_and_run()
 
 
 @click.command(

frappe/parallel_test_runner.py

@@ -26,8 +26,15 @@ class ParallelTestRunner:
 		self.build_number = frappe.utils.cint(build_number) or 1
 		self.total_builds = frappe.utils.cint(total_builds)
 		self.dry_run = dry_run
+		self.test_file_list = []
+		self.total_tests = 0
+		self.test_result = None
+		self.setup_test_file_list()
+
+	def setup_and_run(self):
 		self.setup_test_site()
 		self.run_tests()
+		self.print_result()
 
 	def setup_test_site(self):
 		frappe.init(self.site)
@@ -57,14 +64,17 @@ class ParallelTestRunner:
 		elapsed = click.style(f" ({elapsed:.03}s)", fg="red")
 		click.echo(f"Before Test {elapsed}")
 
+	def setup_test_file_list(self):
+		self.test_file_list = self.get_test_file_list()
+		self.total_tests = sum(self.get_test_count(test) for test in self.test_file_list)
+		click.echo(f"Estimated total tests for build {self.build_number}: {self.total_tests}")
+
 	def run_tests(self):
 		self.test_result = ParallelTestResult(stream=sys.stderr, descriptions=True, verbosity=2)
 
-		for test_file_info in self.get_test_file_list():
+		for test_file_info in self.test_file_list:
 			self.run_tests_for_file(test_file_info)
 
-		self.print_result()
-
 	def run_tests_for_file(self, file_info):
 		if not file_info:
 			return