fix: Breaking backup tests due to site settings

frappe/tests/test_commands.py

@@ -1,4 +1,5 @@
 # Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and Contributors
+# License: MIT. See LICENSE
 
 # imports - standard imports
 import gzip
@@ -9,13 +10,14 @@ import shutil
 import subprocess
 from typing import List
 import unittest
-import glob
+from glob import glob
+from unittest.case import skipIf
 
 # imports - module imports
 import frappe
 import frappe.recorder
 from frappe.installer import add_to_installed_apps, remove_app
-from frappe.utils import add_to_date, get_bench_relative_path, now
+from frappe.utils import add_to_date, get_bench_path, get_bench_relative_path, now
 from frappe.utils.backups import fetch_latest_backups
 
 # imports - third party imports
@@ -277,7 +279,7 @@ class TestCommands(BaseTestCommands):
 		self.assertIsInstance(json.loads(self.stdout), dict)
 
 	def test_get_bench_relative_path(self):
-		bench_path = frappe.utils.get_bench_path()
+		bench_path = get_bench_path()
 		test1_path = os.path.join(bench_path, "test1.txt")
 		test2_path = os.path.join(bench_path, "sites", "test2.txt")
 
@@ -335,7 +337,7 @@ class TestCommands(BaseTestCommands):
 			b"MIT" # app_license
 		]
 		app_name = "testapp0"
-		apps_path = os.path.join(frappe.utils.get_bench_path(), "apps")
+		apps_path = os.path.join(get_bench_path(), "apps")
 		test_app_path = os.path.join(apps_path, app_name)
 		self.execute(f"bench make-app {apps_path} {app_name}", {"cmd_input": b'\n'.join(user_input)})
 		self.assertEqual(self.returncode, 0)
@@ -346,6 +348,10 @@ class TestCommands(BaseTestCommands):
 		# cleanup
 		shutil.rmtree(test_app_path)
 
+	@skipIf(
+		not (frappe.conf.root_password and frappe.conf.admin_password),
+		"DB Root password and Admin password not set in config"
+	)
 	def test_bench_drop_site_should_archive_site(self):
 		site = 'test_site.localhost'
 
@@ -358,7 +364,7 @@ class TestCommands(BaseTestCommands):
 		self.execute(f"bench drop-site {site} --force --root-password {frappe.conf.root_password}")
 		self.assertEqual(self.returncode, 0)
 
-		bench_path = frappe.utils.get_bench_path()
+		bench_path = get_bench_path()
 		site_directory = os.path.join(bench_path, f'sites/{site}')
 		self.assertFalse(os.path.exists(site_directory))
 		archive_directory = os.path.join(bench_path, f'archived/sites/{site}')
@@ -397,17 +403,19 @@ class TestBackups(BaseTestCommands):
 					pass
 
 	def test_backup_no_options(self):
-		# test 1: take a backup
-		before_backup = fetch_latest_backups()
+		"""Take a backup without any options
+		"""
+		before_backup = fetch_latest_backups(partial=True)
 		self.execute("bench --site {site} backup")
-		after_backup = fetch_latest_backups()
+		after_backup = fetch_latest_backups(partial=True)
 
 		self.assertEqual(self.returncode, 0)
 		self.assertIn("successfully completed", self.stdout)
 		self.assertNotEqual(before_backup["database"], after_backup["database"])
 
 	def test_backup_with_files(self):
-		# test 2: take a backup with --with-files
+		"""Take a backup with files (--with-files)
+		"""
 		before_backup = fetch_latest_backups()
 		self.execute("bench --site {site} backup --with-files")
 		after_backup = fetch_latest_backups()
@@ -420,7 +428,8 @@ class TestBackups(BaseTestCommands):
 		self.assertIsNotNone(after_backup["private"])
 
 	def test_backup_with_custom_path(self):
-		# test 3: take a backup with --backup-path
+		"""Backup to a custom path (--backup-path)
+		"""
 		backup_path = os.path.join(self.home, "backups")
 		self.execute("bench --site {site} backup --backup-path {backup_path}", {"backup_path": backup_path})
 
@@ -429,7 +438,8 @@ class TestBackups(BaseTestCommands):
 		self.assertGreaterEqual(len(os.listdir(backup_path)), 2)
 
 	def test_backup_with_different_file_paths(self):
-		# test 4: take a backup with --backup-path-db, --backup-path-files, --backup-path-private-files, --backup-path-conf
+		"""Backup with different file paths (--backup-path-db, --backup-path-files, --backup-path-private-files, --backup-path-conf)
+		"""
 		kwargs = {
 			key: os.path.join(self.home, key, value)
 			for key, value in {
@@ -455,19 +465,22 @@ class TestBackups(BaseTestCommands):
 			self.assertTrue(os.path.exists(path))
 
 	def test_backup_compress_files(self):
-		# test 5: take a backup with --compress
+		"""Take a compressed backup (--compress)
+		"""
 		self.execute("bench --site {site} backup --with-files --compress")
 		self.assertEqual(self.returncode, 0)
-		compressed_files = glob.glob(f"{self.site_backup_path}/*.tgz")
+		compressed_files = glob(f"{self.site_backup_path}/*.tgz")
 		self.assertGreater(len(compressed_files), 0)
 
 	def test_backup_verbose(self):
-		# test 6: take a backup with --verbose
+		"""Take a verbose backup (--verbose)
+		"""
 		self.execute("bench --site {site} backup --verbose")
 		self.assertEqual(self.returncode, 0)
 
 	def test_backup_only_specific_doctypes(self):
-		# test 7: take a backup with frappe.conf.backup.includes
+		"""Take a backup with (include) backup options set in the site config `frappe.conf.backup.includes`
+		"""
 		self.execute(
 			"bench --site {site} set-config backup '{includes}' --parse",
 			{"includes": json.dumps(self.backup_map["includes"])},
@@ -478,7 +491,9 @@ class TestBackups(BaseTestCommands):
 		self.assertEqual([], missing_in_backup(self.backup_map["includes"]["includes"], database))
 
 	def test_backup_excluding_specific_doctypes(self):
-		# test 8: take a backup with frappe.conf.backup.excludes
+		"""Take a backup with (exclude) backup options set (`frappe.conf.backup.excludes`, `--exclude`)
+		"""
+		# test 1: take a backup with frappe.conf.backup.excludes
 		self.execute(
 			"bench --site {site} set-config backup '{excludes}' --parse",
 			{"excludes": json.dumps(self.backup_map["excludes"])},
@@ -489,7 +504,7 @@ class TestBackups(BaseTestCommands):
 		self.assertFalse(exists_in_backup(self.backup_map["excludes"]["excludes"], database))
 		self.assertEqual([], missing_in_backup(self.backup_map["includes"]["includes"], database))
 
-		# test 9: take a backup with --exclude
+		# test 2: take a backup with --exclude
 		self.execute(
 			"bench --site {site} backup --exclude '{exclude}'",
 			{"exclude": ",".join(self.backup_map["excludes"]["excludes"])},
@@ -499,7 +514,8 @@ class TestBackups(BaseTestCommands):
 		self.assertFalse(exists_in_backup(self.backup_map["excludes"]["excludes"], database))
 
 	def test_selective_backup_priority_resolution(self):
-		# test 10: take a backup with --include (with frappe.conf.excludes still set)
+		"""Take a backup with conflicting backup options set (`frappe.conf.excludes`, `--include`)
+		"""
 		self.execute(
 			"bench --site {site} backup --include '{include}'",
 			{"include": ",".join(self.backup_map["includes"]["includes"])},
@@ -509,7 +525,8 @@ class TestBackups(BaseTestCommands):
 		self.assertEqual([], missing_in_backup(self.backup_map["includes"]["includes"], database))
 
 	def test_dont_backup_conf(self):
-		# test 11: take a backup with --ignore-backup-conf
+		"""Take a backup ignoring frappe.conf.backup settings (with --ignore-backup-conf option)
+		"""
 		self.execute("bench --site {site} backup --ignore-backup-conf")
 		self.assertEqual(self.returncode, 0)
 		database = fetch_latest_backups()["database"]