From 820884146013abaee1a53378f32690239858b148 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 2 May 2024 12:10:19 +0200 Subject: [PATCH 1/9] common: make explicit which test cases need mocked options --- tests/common.TestCase | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/tests/common.TestCase b/tests/common.TestCase index 24132297..11ec5d7b 100755 --- a/tests/common.TestCase +++ b/tests/common.TestCase @@ -45,6 +45,12 @@ from fdroidserver.exception import FDroidException, VCSException,\ from fdroidserver.looseversion import LooseVersion +def _mock_common_module_options_instance(): + """Helper method to deal with difficult visibility of the module-level options.""" + fdroidserver.common.options = mock.Mock() + fdroidserver.common.options.verbose = False + + class CommonTest(unittest.TestCase): '''fdroidserver/common.py''' @@ -57,14 +63,18 @@ class CommonTest(unittest.TestCase): if not os.path.exists(self.tmpdir): os.makedirs(self.tmpdir) os.chdir(self.basedir) + + # these are declared as None at the top of the module file fdroidserver.common.config = None - fdroidserver.common.options = mock.Mock() - fdroidserver.common.options.verbose = False + fdroidserver.common.options = None fdroidserver.metadata.srclibs = None + self._td = mkdtemp() self.testdir = self._td.name def tearDown(self): + fdroidserver.common.config = None + fdroidserver.common.options = None os.chdir(self.basedir) self._td.cleanup() shutil.rmtree(self.tmpdir) @@ -356,6 +366,7 @@ class CommonTest(unittest.TestCase): config = dict() fdroidserver.common.fill_config_defaults(config) fdroidserver.common.config = config + _mock_common_module_options_instance() srclibname = 'FakeSrcLib' srclib_testdir = os.path.join(self.testdir, 'build', 'srclib') @@ -397,6 +408,7 @@ class CommonTest(unittest.TestCase): onserver=True, refresh=False) # do not clone in this test def test_prepare_sources_refresh(self): + _mock_common_module_options_instance() packageName = 'org.fdroid.ci.test.app' os.chdir(self.tmpdir) os.mkdir('build') @@ -467,6 +479,7 @@ class CommonTest(unittest.TestCase): config = dict() fdroidserver.common.fill_config_defaults(config) fdroidserver.common.config = config + _mock_common_module_options_instance() commands = ['sh', '-c', 'echo stdout message && echo stderr message 1>&2'] @@ -477,6 +490,7 @@ class CommonTest(unittest.TestCase): self.assertEqual(p.output, 'stdout message\n') def test_signjar(self): + _mock_common_module_options_instance() config = fdroidserver.common.read_config(fdroidserver.common.options) config['jarsigner'] = fdroidserver.common.find_sdk_tools_cmd('jarsigner') fdroidserver.common.config = config @@ -497,6 +511,7 @@ class CommonTest(unittest.TestCase): ) def test_verify_apk_signature(self): + _mock_common_module_options_instance() config = fdroidserver.common.read_config(fdroidserver.common.options) fdroidserver.common.config = config @@ -519,6 +534,7 @@ class CommonTest(unittest.TestCase): self.assertFalse(fdroidserver.common.verify_apk_signature('urzip-release-unsigned.apk')) def test_verify_old_apk_signature(self): + _mock_common_module_options_instance() config = fdroidserver.common.read_config(fdroidserver.common.options) config['jarsigner'] = fdroidserver.common.find_sdk_tools_cmd('jarsigner') fdroidserver.common.config = config @@ -583,6 +599,7 @@ class CommonTest(unittest.TestCase): def test_verify_apks(self): config = fdroidserver.common.read_config(fdroidserver.common.options) fdroidserver.common.config = config + _mock_common_module_options_instance() sourceapk = os.path.join(self.basedir, 'urzip.apk') @@ -889,6 +906,7 @@ class CommonTest(unittest.TestCase): ) def test_sign_apk(self): + _mock_common_module_options_instance() config = fdroidserver.common.read_config(fdroidserver.common.options) if 'apksigner' not in config: self.skipTest('SKIPPING test_sign_apk, apksigner not installed!') @@ -959,6 +977,7 @@ class CommonTest(unittest.TestCase): @unittest.skipIf(os.getuid() == 0, 'This is meaningless when run as root') def test_sign_apk_fail(self): + _mock_common_module_options_instance() config = fdroidserver.common.read_config(fdroidserver.common.options) if 'apksigner' not in config: self.skipTest('SKIPPING test_sign_apk_fail, apksigner not installed!') @@ -982,6 +1001,7 @@ class CommonTest(unittest.TestCase): self.assertFalse(os.path.isfile(signed)) def test_sign_apk_corrupt(self): + _mock_common_module_options_instance() config = fdroidserver.common.read_config(fdroidserver.common.options) if 'apksigner' not in config: self.skipTest('SKIPPING test_sign_apk_corrupt, apksigner not installed!') @@ -1008,6 +1028,7 @@ class CommonTest(unittest.TestCase): ) def test_resign_apk(self): """When using apksigner, it should resign signed APKs""" + _mock_common_module_options_instance() config = fdroidserver.common.read_config(fdroidserver.common.options) if 'apksigner' not in config: self.skipTest('SKIPPING test_resign_apk, apksigner not installed!') @@ -2532,6 +2553,7 @@ class CommonTest(unittest.TestCase): @mock.patch.dict(os.environ, clear=True) def test_FDroidPopen_envs_paths_can_be_pathlib(self): + _mock_common_module_options_instance() os.environ['PATH'] = '/usr/bin:/usr/sbin' envs = {'PATHLIB': Path('/pathlib/path'), 'STRING': '/string/path'} p = fdroidserver.common.FDroidPopen(['/bin/sh', '-c', 'export'], envs=envs) From 1eaba25021197b4f0dea81bc1106bd5b584a9079 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 2 May 2024 12:11:33 +0200 Subject: [PATCH 2/9] common: do not use module reference for local functions This just makes things more confusing. --- fdroidserver/common.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fdroidserver/common.py b/fdroidserver/common.py index b3e41c81..22af3e00 100644 --- a/fdroidserver/common.py +++ b/fdroidserver/common.py @@ -380,7 +380,7 @@ def get_config(opts=None): if config is not None: return config - common.read_config(opts=opts) + read_config(opts=opts) # make sure these values are available in common.py even if they didn't # declare global in a scope @@ -570,7 +570,7 @@ def parse_mirrors_config(mirrors): def file_entry(filename, hash_value=None): meta = {} meta["name"] = "/" + Path(filename).as_posix().split("/", 1)[1] - meta["sha256"] = hash_value or common.sha256sum(filename) + meta["sha256"] = hash_value or sha256sum(filename) meta["size"] = os.stat(filename).st_size return meta From 92a3f4b19103767b4f56c364ac947e4d4b776309 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 2 May 2024 12:19:02 +0200 Subject: [PATCH 3/9] rename local variable to stop overwriting global options This fixes a bug where if smartcardoptions is set as a str in config.yml will overwrite all command line options. a4d069862 fdroidserver!1106 --- fdroidserver/common.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fdroidserver/common.py b/fdroidserver/common.py index 22af3e00..1f51fd21 100644 --- a/fdroidserver/common.py +++ b/fdroidserver/common.py @@ -477,8 +477,8 @@ def read_config(opts=None): # smartcardoptions must be a list since its command line args for Popen smartcardoptions = config.get('smartcardoptions') if isinstance(smartcardoptions, str): - options = re.sub(r'\s+', r' ', config['smartcardoptions']).split(' ') - config['smartcardoptions'] = [i.strip() for i in options if i] + sco_items = re.sub(r'\s+', r' ', config['smartcardoptions']).split(' ') + config['smartcardoptions'] = [i.strip() for i in sco_items if i] elif not smartcardoptions and 'keystore' in config and config['keystore'] == 'NONE': # keystore='NONE' means use smartcard, these are required defaults config['smartcardoptions'] = ['-storetype', 'PKCS11', '-providerName', From 717df09be020e3a3ac613a0f5e5625263c7f08b1 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 2 May 2024 14:05:56 +0200 Subject: [PATCH 4/9] clarify that config/options can be global or module-level variable --- fdroidserver/common.py | 22 ++++++++++++++++- tests/common.TestCase | 54 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/fdroidserver/common.py b/fdroidserver/common.py index 1f51fd21..4074d2aa 100644 --- a/fdroidserver/common.py +++ b/fdroidserver/common.py @@ -374,7 +374,27 @@ def fill_config_defaults(thisconfig): def get_config(opts=None): - """Get config instace. This function takes care of initializing config data before returning it.""" + """Get the initalized, singleton config instance. + + config and options are intertwined in read_config(), so they have + to be here too. In the current ugly state of things, there are + multiple potential instances of config and options in use: + + * global + * module-level in the subcommand module (e.g. fdroidserver/build.py) + * module-level in fdroidserver.common + + There are some insane parts of the code that are probably + referring to multiple instances of these at different points. + This can be super confusing and maddening. + + The current intermediate refactoring step is to move all + subcommands to always get/set config and options via this function + so that there is no longer a distinction between the global and + module-level instances. Then there can be only one module-level + instance in fdroidserver.common. + + """ global config, options if config is not None: diff --git a/tests/common.TestCase b/tests/common.TestCase index 11ec5d7b..a162826e 100755 --- a/tests/common.TestCase +++ b/tests/common.TestCase @@ -3237,6 +3237,60 @@ class SignerExtractionTest(unittest.TestCase): ) +class ConfigOptionsScopeTest(unittest.TestCase): + """Test assumptions about variable scope for "config" and "options". + + The ancient architecture of config and options in fdroidserver has + weird issues around unexpected scope, like there are cases where + the global config is not the same as the module-level config, and + more. + + This is about describing what is happening, it is not about + documenting behaviors that are good design. The config and options + handling should really be refactored into a well-known, workable + Pythonic pattern. + + """ + + def setUp(self): + # these are declared as None at the top of the module file + fdroidserver.common.config = None + fdroidserver.common.options = None + + def tearDown(self): + fdroidserver.common.config = None + fdroidserver.common.options = None + if 'config' in globals(): + global config + del config + if 'options' in globals(): + global options + del options + + def test_get_config(self): + """Show how the module-level variables are initialized.""" + self.assertTrue('config' not in vars() and 'config' not in globals()) + self.assertIsNone(fdroidserver.common.config) + config = fdroidserver.common.get_config() + self.assertIsNotNone(fdroidserver.common.config) + self.assertEqual(dict, type(config)) + self.assertEqual(config, fdroidserver.common.config) + + def test_get_config_global(self): + """Test assumptions about variable scope using global keyword.""" + global config + self.assertTrue('config' not in vars() and 'config' not in globals()) + self.assertIsNone(fdroidserver.common.config) + c = fdroidserver.common.get_config() + self.assertIsNotNone(fdroidserver.common.config) + self.assertEqual(dict, type(c)) + self.assertEqual(c, fdroidserver.common.config) + self.assertTrue( + 'config' not in vars() and 'config' not in globals(), + "The config should not be set in the global context, only module-level.", + ) + + if __name__ == "__main__": os.chdir(os.path.dirname(__file__)) From 1e5699e90ce031fb62fc269ee6da262a3865ce2d Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 2 May 2024 14:08:28 +0200 Subject: [PATCH 5/9] remove all references to optparse (deprecated since Python 3.2) --- fdroidserver/__main__.py | 2 +- tests/build.TestCase | 9 +++++---- tests/checkupdates.TestCase | 9 +++++---- tests/common.TestCase | 8 ++++---- tests/deploy.TestCase | 9 +++++---- tests/exception.TestCase | 9 +++++---- tests/gpgsign.TestCase | 9 +++++---- tests/import_subcommand.TestCase | 9 +++++---- tests/index.TestCase | 9 +++++---- tests/init.TestCase | 9 +++++---- tests/install.TestCase | 9 +++++---- tests/lint.TestCase | 9 +++++---- tests/main.TestCase | 9 +++++---- tests/metadata.TestCase | 9 +++++---- tests/net.TestCase | 9 +++++---- tests/nightly.TestCase | 9 +++++---- tests/publish.TestCase | 9 +++++---- tests/rewritemeta.TestCase | 9 +++++---- tests/scanner.TestCase | 9 +++++---- tests/signatures.TestCase | 9 +++++---- tests/signindex.TestCase | 9 +++++---- tests/update.TestCase | 9 +++++---- tests/vcs.TestCase | 9 +++++---- tests/verify.TestCase | 9 +++++---- 24 files changed, 115 insertions(+), 93 deletions(-) diff --git a/fdroidserver/__main__.py b/fdroidserver/__main__.py index 00c19e35..14813fa1 100755 --- a/fdroidserver/__main__.py +++ b/fdroidserver/__main__.py @@ -182,7 +182,7 @@ def main(): "can not be specified at the same time.")) sys.exit(1) - # Trick optparse into displaying the right usage when --help is used. + # Trick argparse into displaying the right usage when --help is used. sys.argv[0] += ' ' + command del sys.argv[1] diff --git a/tests/build.TestCase b/tests/build.TestCase index 3ad4cdd0..0228bf35 100755 --- a/tests/build.TestCase +++ b/tests/build.TestCase @@ -4,7 +4,6 @@ import inspect import logging -import optparse import os import shutil import sys @@ -1104,15 +1103,17 @@ class BuildTest(unittest.TestCase): if __name__ == "__main__": os.chdir(os.path.dirname(__file__)) - parser = optparse.OptionParser() - parser.add_option( + import argparse + + parser = argparse.ArgumentParser() + parser.add_argument( "-v", "--verbose", action="store_true", default=False, help="Spew out even more information than normal", ) - (fdroidserver.common.options, args) = parser.parse_args(['--verbose']) + fdroidserver.common.options = parser.parse_args(['--verbose']) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(BuildTest)) diff --git a/tests/checkupdates.TestCase b/tests/checkupdates.TestCase index 972f1eef..0ae895de 100755 --- a/tests/checkupdates.TestCase +++ b/tests/checkupdates.TestCase @@ -3,7 +3,6 @@ # http://www.drdobbs.com/testing/unit-testing-with-python/240165163 import logging -import optparse import os import sys import unittest @@ -336,15 +335,17 @@ class CheckupdatesTest(unittest.TestCase): if __name__ == "__main__": - parser = optparse.OptionParser() - parser.add_option( + import argparse + + parser = argparse.ArgumentParser() + parser.add_argument( "-v", "--verbose", action="store_true", default=False, help="Spew out even more information than normal", ) - (fdroidserver.common.options, args) = parser.parse_args(['--verbose']) + fdroidserver.common.options = parser.parse_args(['--verbose']) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(CheckupdatesTest)) diff --git a/tests/common.TestCase b/tests/common.TestCase index a162826e..b3e32100 100755 --- a/tests/common.TestCase +++ b/tests/common.TestCase @@ -9,7 +9,6 @@ import importlib import inspect import json import logging -import optparse import os import re import ruamel.yaml @@ -22,6 +21,7 @@ import unittest import textwrap import yaml import gzip +from argparse import ArgumentParser from zipfile import BadZipFile, ZipFile from unittest import mock from pathlib import Path @@ -3294,15 +3294,15 @@ class ConfigOptionsScopeTest(unittest.TestCase): if __name__ == "__main__": os.chdir(os.path.dirname(__file__)) - parser = optparse.OptionParser() - parser.add_option( + parser = ArgumentParser() + parser.add_argument( "-v", "--verbose", action="store_true", default=False, help="Spew out even more information than normal", ) - (fdroidserver.common.options, args) = parser.parse_args(['--verbose']) + fdroidserver.common.options = parser.parse_args(['--verbose']) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(CommonTest)) diff --git a/tests/deploy.TestCase b/tests/deploy.TestCase index fd17d062..f2082dd3 100755 --- a/tests/deploy.TestCase +++ b/tests/deploy.TestCase @@ -2,7 +2,6 @@ import inspect import logging -import optparse import os import sys import tempfile @@ -390,15 +389,17 @@ class DeployTest(unittest.TestCase): if __name__ == "__main__": os.chdir(os.path.dirname(__file__)) - parser = optparse.OptionParser() - parser.add_option( + import argparse + + parser = argparse.ArgumentParser() + parser.add_argument( "-v", "--verbose", action="store_true", default=False, help="Spew out even more information than normal", ) - (fdroidserver.common.options, args) = parser.parse_args(['--verbose']) + fdroidserver.common.options = parser.parse_args(['--verbose']) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(DeployTest)) diff --git a/tests/exception.TestCase b/tests/exception.TestCase index a1d8a992..bfa44f57 100755 --- a/tests/exception.TestCase +++ b/tests/exception.TestCase @@ -3,7 +3,6 @@ # http://www.drdobbs.com/testing/unit-testing-with-python/240165163 import inspect -import optparse import os import sys import unittest @@ -57,15 +56,17 @@ class ExceptionTest(unittest.TestCase): if __name__ == "__main__": os.chdir(os.path.dirname(__file__)) - parser = optparse.OptionParser() - parser.add_option( + import argparse + + parser = argparse.ArgumentParser() + parser.add_argument( "-v", "--verbose", action="store_true", default=False, help="Spew out even more information than normal", ) - (fdroidserver.exception.options, args) = parser.parse_args(['--verbose']) + fdroidserver.exception.options = parser.parse_args(['--verbose']) fdroidserver.common.options = fdroidserver.exception.options newSuite = unittest.TestSuite() diff --git a/tests/gpgsign.TestCase b/tests/gpgsign.TestCase index ef5ea007..27feb79e 100755 --- a/tests/gpgsign.TestCase +++ b/tests/gpgsign.TestCase @@ -3,7 +3,6 @@ import inspect import json import logging -import optparse import os import shutil import sys @@ -75,15 +74,17 @@ class GpgsignTest(unittest.TestCase): if __name__ == "__main__": os.chdir(os.path.dirname(__file__)) - parser = optparse.OptionParser() - parser.add_option( + import argparse + + parser = argparse.ArgumentParser() + parser.add_argument( "-v", "--verbose", action="store_true", default=False, help="Spew out even more information than normal", ) - (common.options, args) = parser.parse_args(['--verbose']) + common.options = parser.parse_args(['--verbose']) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(GpgsignTest)) diff --git a/tests/import_subcommand.TestCase b/tests/import_subcommand.TestCase index eb41ae20..79222eab 100755 --- a/tests/import_subcommand.TestCase +++ b/tests/import_subcommand.TestCase @@ -4,7 +4,6 @@ import git import logging -import optparse import os import shutil import sys @@ -161,15 +160,17 @@ class ImportTest(unittest.TestCase): if __name__ == "__main__": - parser = optparse.OptionParser() - parser.add_option( + import argparse + + parser = argparse.ArgumentParser() + parser.add_argument( "-v", "--verbose", action="store_true", default=False, help="Spew out even more information than normal", ) - (fdroidserver.common.options, args) = parser.parse_args(['--verbose']) + fdroidserver.common.options = parser.parse_args(['--verbose']) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(ImportTest)) diff --git a/tests/index.TestCase b/tests/index.TestCase index 3fb4a034..3e70acb1 100755 --- a/tests/index.TestCase +++ b/tests/index.TestCase @@ -5,7 +5,6 @@ import datetime import glob import inspect import logging -import optparse import os import sys import unittest @@ -942,15 +941,17 @@ class AltstoreIndexTest(unittest.TestCase): if __name__ == "__main__": os.chdir(os.path.dirname(__file__)) - parser = optparse.OptionParser() - parser.add_option( + import argparse + + parser = argparse.ArgumentParser() + parser.add_argument( "-v", "--verbose", action="store_true", default=False, help="Spew out even more information than normal", ) - (options, args) = parser.parse_args(["--verbose"]) + options = parser.parse_args(["--verbose"]) Options.verbose = options.verbose newSuite = unittest.TestSuite() diff --git a/tests/init.TestCase b/tests/init.TestCase index 8e1042c3..9266fa7c 100755 --- a/tests/init.TestCase +++ b/tests/init.TestCase @@ -5,7 +5,6 @@ import inspect import logging import os -import optparse import shutil import sys import unittest @@ -75,15 +74,17 @@ class InitTest(unittest.TestCase): if __name__ == "__main__": os.chdir(os.path.dirname(__file__)) - parser = optparse.OptionParser() - parser.add_option( + import argparse + + parser = argparse.ArgumentParser() + parser.add_argument( "-v", "--verbose", action="store_true", default=False, help="Spew out even more information than normal", ) - (fdroidserver.init.options, args) = parser.parse_args(['--verbose']) + fdroidserver.init.options = parser.parse_args(['--verbose']) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(InitTest)) diff --git a/tests/install.TestCase b/tests/install.TestCase index c2fad222..2f4569ea 100755 --- a/tests/install.TestCase +++ b/tests/install.TestCase @@ -3,7 +3,6 @@ # http://www.drdobbs.com/testing/unit-testing-with-python/240165163 import inspect -import optparse import os import sys import unittest @@ -38,15 +37,17 @@ class InstallTest(unittest.TestCase): if __name__ == "__main__": os.chdir(os.path.dirname(__file__)) - parser = optparse.OptionParser() - parser.add_option( + import argparse + + parser = argparse.ArgumentParser() + parser.add_argument( "-v", "--verbose", action="store_true", default=False, help="Spew out even more information than normal", ) - (fdroidserver.install.options, args) = parser.parse_args(['--verbose']) + fdroidserver.install.options = parser.parse_args(['--verbose']) fdroidserver.common.options = fdroidserver.install.options newSuite = unittest.TestSuite() diff --git a/tests/lint.TestCase b/tests/lint.TestCase index 5dd94d4b..5430eb7b 100755 --- a/tests/lint.TestCase +++ b/tests/lint.TestCase @@ -3,7 +3,6 @@ # http://www.drdobbs.com/testing/unit-testing-with-python/240165163 import logging -import optparse import os import ruamel.yaml import shutil @@ -527,15 +526,17 @@ class LintAntiFeaturesTest(unittest.TestCase): if __name__ == "__main__": - parser = optparse.OptionParser() - parser.add_option( + import argparse + + parser = argparse.ArgumentParser() + parser.add_argument( "-v", "--verbose", action="store_true", default=False, help="Spew out even more information than normal", ) - (fdroidserver.lint.options, args) = parser.parse_args(['--verbose']) + fdroidserver.lint.options = parser.parse_args(['--verbose']) fdroidserver.common.options = fdroidserver.lint.options newSuite = unittest.TestSuite() diff --git a/tests/main.TestCase b/tests/main.TestCase index 3809fe17..deaf9dff 100755 --- a/tests/main.TestCase +++ b/tests/main.TestCase @@ -1,7 +1,6 @@ #!/usr/bin/env python3 import inspect -import optparse import os import sys import pkgutil @@ -265,15 +264,17 @@ class MainTest(unittest.TestCase): if __name__ == "__main__": os.chdir(os.path.dirname(__file__)) - parser = optparse.OptionParser() - parser.add_option( + import argparse + + parser = argparse.ArgumentParser() + parser.add_argument( "-v", "--verbose", action="store_true", default=False, help="Spew out even more information than normal", ) - (common.options, args) = parser.parse_args(['--verbose']) + common.options = parser.parse_args(['--verbose']) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(MainTest)) diff --git a/tests/metadata.TestCase b/tests/metadata.TestCase index c46d5bbf..c4c58882 100755 --- a/tests/metadata.TestCase +++ b/tests/metadata.TestCase @@ -3,7 +3,6 @@ import copy import io import logging -import optparse import os import random import ruamel.yaml @@ -2447,15 +2446,17 @@ class PostMetadataParseTest(unittest.TestCase): if __name__ == "__main__": - parser = optparse.OptionParser() - parser.add_option( + import argparse + + parser = argparse.ArgumentParser() + parser.add_argument( "-v", "--verbose", action="store_true", default=False, help="Spew out even more information than normal", ) - (fdroidserver.common.options, args) = parser.parse_args(['--verbose']) + fdroidserver.common.options = parser.parse_args(['--verbose']) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(MetadataTest)) diff --git a/tests/net.TestCase b/tests/net.TestCase index 94b8ce84..59b299b8 100755 --- a/tests/net.TestCase +++ b/tests/net.TestCase @@ -2,7 +2,6 @@ import inspect import logging -import optparse import os import random import requests @@ -126,15 +125,17 @@ class NetTest(unittest.TestCase): if __name__ == "__main__": os.chdir(os.path.dirname(__file__)) - parser = optparse.OptionParser() - parser.add_option( + import argparse + + parser = argparse.ArgumentParser() + parser.add_argument( "-v", "--verbose", action="store_true", default=False, help="Spew out even more information than normal", ) - (common.options, args) = parser.parse_args(['--verbose']) + common.options = parser.parse_args(['--verbose']) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(NetTest)) diff --git a/tests/nightly.TestCase b/tests/nightly.TestCase index 9e505c7f..7cb1712d 100755 --- a/tests/nightly.TestCase +++ b/tests/nightly.TestCase @@ -2,7 +2,6 @@ import inspect import logging -import optparse import os import requests import shutil @@ -364,15 +363,17 @@ class NightlyTest(unittest.TestCase): if __name__ == "__main__": os.chdir(os.path.dirname(__file__)) - parser = optparse.OptionParser() - parser.add_option( + import argparse + + parser = argparse.ArgumentParser() + parser.add_argument( "-v", "--verbose", action="store_true", default=False, help="Spew out even more information than normal", ) - (common.options, args) = parser.parse_args(['--verbose']) + common.options = parser.parse_args(['--verbose']) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(NightlyTest)) diff --git a/tests/publish.TestCase b/tests/publish.TestCase index 80556744..00bbec0e 100755 --- a/tests/publish.TestCase +++ b/tests/publish.TestCase @@ -13,7 +13,6 @@ import inspect import json import logging -import optparse import os import shutil import sys @@ -413,15 +412,17 @@ class PublishTest(unittest.TestCase): if __name__ == "__main__": os.chdir(os.path.dirname(__file__)) - parser = optparse.OptionParser() - parser.add_option( + import argparse + + parser = argparse.ArgumentParser() + parser.add_argument( "-v", "--verbose", action="store_true", default=False, help="Spew out even more information than normal", ) - (common.options, args) = parser.parse_args(['--verbose']) + common.options = parser.parse_args(['--verbose']) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(PublishTest)) diff --git a/tests/rewritemeta.TestCase b/tests/rewritemeta.TestCase index 9d20550f..00e4f354 100755 --- a/tests/rewritemeta.TestCase +++ b/tests/rewritemeta.TestCase @@ -1,7 +1,6 @@ #!/usr/bin/env python3 import logging -import optparse import os import sys import unittest @@ -265,15 +264,17 @@ class RewriteMetaTest(unittest.TestCase): if __name__ == "__main__": - parser = optparse.OptionParser() - parser.add_option( + import argparse + + parser = argparse.ArgumentParser() + parser.add_argument( "-v", "--verbose", action="store_true", default=False, help="Spew out even more information than normal", ) - (common.options, args) = parser.parse_args(['--verbose']) + common.options = parser.parse_args(['--verbose']) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(RewriteMetaTest)) diff --git a/tests/scanner.TestCase b/tests/scanner.TestCase index 83ce75b7..9069164e 100755 --- a/tests/scanner.TestCase +++ b/tests/scanner.TestCase @@ -3,7 +3,6 @@ import glob import inspect import logging -import optparse import os import re import shutil @@ -815,15 +814,17 @@ class Test_main(unittest.TestCase): if __name__ == "__main__": os.chdir(os.path.dirname(__file__)) - parser = optparse.OptionParser() - parser.add_option( + import argparse + + parser = argparse.ArgumentParser() + parser.add_argument( "-v", "--verbose", action="store_true", default=False, help="Spew out even more information than normal", ) - (fdroidserver.common.options, args) = parser.parse_args(['--verbose']) + fdroidserver.common.options = parser.parse_args(['--verbose']) newSuite = unittest.TestSuite() newSuite.addTests( diff --git a/tests/signatures.TestCase b/tests/signatures.TestCase index 4744613b..5d043a9f 100755 --- a/tests/signatures.TestCase +++ b/tests/signatures.TestCase @@ -1,7 +1,6 @@ #!/usr/bin/env python3 import inspect -import optparse import os import sys import unittest @@ -59,15 +58,17 @@ class SignaturesTest(unittest.TestCase): if __name__ == "__main__": os.chdir(os.path.dirname(__file__)) - parser = optparse.OptionParser() - parser.add_option( + import argparse + + parser = argparse.ArgumentParser() + parser.add_argument( "-v", "--verbose", action="store_true", default=False, help="Spew out even more information than normal", ) - (common.options, args) = parser.parse_args(['--verbose']) + common.options = parser.parse_args(['--verbose']) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(SignaturesTest)) diff --git a/tests/signindex.TestCase b/tests/signindex.TestCase index 28afa689..7931fbdc 100755 --- a/tests/signindex.TestCase +++ b/tests/signindex.TestCase @@ -3,7 +3,6 @@ import inspect import json import logging -import optparse import os import shutil import subprocess @@ -193,15 +192,17 @@ class SignindexTest(unittest.TestCase): if __name__ == "__main__": os.chdir(os.path.dirname(__file__)) - parser = optparse.OptionParser() - parser.add_option( + import argparse + + parser = argparse.ArgumentParser() + parser.add_argument( "-v", "--verbose", action="store_true", default=False, help="Spew out even more information than normal", ) - (common.options, args) = parser.parse_args(['--verbose']) + common.options = parser.parse_args(['--verbose']) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(SignindexTest)) diff --git a/tests/update.TestCase b/tests/update.TestCase index 0c692aa8..6af32bb2 100755 --- a/tests/update.TestCase +++ b/tests/update.TestCase @@ -9,7 +9,6 @@ import hashlib import inspect import json import logging -import optparse import os import random import shutil @@ -2271,15 +2270,17 @@ class TestParseFromPbxproj(unittest.TestCase): if __name__ == "__main__": os.chdir(os.path.dirname(__file__)) - parser = optparse.OptionParser() - parser.add_option( + import argparse + + parser = argparse.ArgumentParser() + parser.add_argument( "-v", "--verbose", action="store_true", default=False, help="Spew out even more information than normal", ) - (fdroidserver.common.options, args) = parser.parse_args(['--verbose']) + fdroidserver.common.options = parser.parse_args(['--verbose']) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(UpdateTest)) diff --git a/tests/vcs.TestCase b/tests/vcs.TestCase index 2e46f926..99af14cc 100755 --- a/tests/vcs.TestCase +++ b/tests/vcs.TestCase @@ -4,7 +4,6 @@ import inspect import logging -import optparse import os import sys import unittest @@ -88,15 +87,17 @@ class VCSTest(unittest.TestCase): if __name__ == "__main__": os.chdir(os.path.dirname(__file__)) - parser = optparse.OptionParser() - parser.add_option( + import argparse + + parser = argparse.ArgumentParser() + parser.add_argument( "-v", "--verbose", action="store_true", default=False, help="Spew out even more information than normal", ) - (fdroidserver.common.options, args) = parser.parse_args(['--verbose']) + fdroidserver.common.options = parser.parse_args(['--verbose']) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(VCSTest)) diff --git a/tests/verify.TestCase b/tests/verify.TestCase index 774e24fa..cd8fbb6e 100755 --- a/tests/verify.TestCase +++ b/tests/verify.TestCase @@ -3,7 +3,6 @@ import inspect import json import logging -import optparse import os import shutil import sys @@ -94,15 +93,17 @@ class VerifyTest(unittest.TestCase): if __name__ == "__main__": os.chdir(os.path.dirname(__file__)) - parser = optparse.OptionParser() - parser.add_option( + import argparse + + parser = argparse.ArgumentParser() + parser.add_argument( "-v", "--verbose", action="store_true", default=False, help="Spew out even more information than normal", ) - (common.options, args) = parser.parse_args(['--verbose']) + common.options = parser.parse_args(['--verbose']) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(VerifyTest)) From 685efa23d43aedbc8b2d1dc7630abdc4f709e2e6 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Mon, 6 May 2024 11:13:13 +0200 Subject: [PATCH 6/9] import: always load testcommon from localmodule Having this import before sys.path.insert() made it load testcommon from the Debian package. --- tests/import_subcommand.TestCase | 2 +- tests/lint.TestCase | 2 +- tests/main.TestCase | 2 +- tests/metadata.TestCase | 3 +-- tests/rewritemeta.TestCase | 3 +-- tests/update.TestCase | 2 +- 6 files changed, 6 insertions(+), 8 deletions(-) diff --git a/tests/import_subcommand.TestCase b/tests/import_subcommand.TestCase index 79222eab..1b3347ad 100755 --- a/tests/import_subcommand.TestCase +++ b/tests/import_subcommand.TestCase @@ -14,7 +14,6 @@ from unittest import mock from pathlib import Path import requests -from testcommon import TmpCwd, mkdtemp localmodule = Path(__file__).resolve().parent.parent print('localmodule: ' + str(localmodule)) @@ -25,6 +24,7 @@ import fdroidserver.common import fdroidserver.import_subcommand import fdroidserver.metadata from fdroidserver.exception import FDroidException +from testcommon import TmpCwd, mkdtemp class ImportTest(unittest.TestCase): diff --git a/tests/lint.TestCase b/tests/lint.TestCase index 5430eb7b..d9ac8de2 100755 --- a/tests/lint.TestCase +++ b/tests/lint.TestCase @@ -10,7 +10,6 @@ import sys import tempfile import unittest from pathlib import Path -from testcommon import mkdtemp localmodule = Path(__file__).resolve().parent.parent print('localmodule: ' + str(localmodule)) @@ -21,6 +20,7 @@ import fdroidserver.common import fdroidserver.lint import fdroidserver.metadata from fdroidserver.common import CATEGORIES_CONFIG_NAME +from testcommon import mkdtemp class LintTest(unittest.TestCase): diff --git a/tests/main.TestCase b/tests/main.TestCase index deaf9dff..9ce25a2b 100755 --- a/tests/main.TestCase +++ b/tests/main.TestCase @@ -8,7 +8,6 @@ import textwrap import unittest import tempfile from unittest import mock -from testcommon import TmpCwd, TmpPyPath localmodule = os.path.realpath( os.path.join(os.path.dirname(inspect.getfile(inspect.currentframe())), '..') @@ -19,6 +18,7 @@ if localmodule not in sys.path: from fdroidserver import common import fdroidserver.__main__ +from testcommon import TmpCwd, TmpPyPath class MainTest(unittest.TestCase): diff --git a/tests/metadata.TestCase b/tests/metadata.TestCase index c4c58882..1a0a1bdc 100755 --- a/tests/metadata.TestCase +++ b/tests/metadata.TestCase @@ -15,8 +15,6 @@ from collections import OrderedDict from pathlib import Path from unittest import mock -from testcommon import TmpCwd, mkdtemp - localmodule = Path(__file__).resolve().parent.parent print('localmodule: ' + str(localmodule)) if localmodule not in sys.path: @@ -26,6 +24,7 @@ import fdroidserver from fdroidserver import metadata from fdroidserver.exception import MetaDataException from fdroidserver.common import DEFAULT_LOCALE +from testcommon import TmpCwd, mkdtemp def _get_mock_mf(s): diff --git a/tests/rewritemeta.TestCase b/tests/rewritemeta.TestCase index 00e4f354..f8388f88 100755 --- a/tests/rewritemeta.TestCase +++ b/tests/rewritemeta.TestCase @@ -8,14 +8,13 @@ import tempfile import textwrap from pathlib import Path -from testcommon import TmpCwd, mkdtemp - localmodule = Path(__file__).resolve().parent.parent print('localmodule: ' + str(localmodule)) if localmodule not in sys.path: sys.path.insert(0, str(localmodule)) from fdroidserver import common, metadata, rewritemeta +from testcommon import TmpCwd, mkdtemp class RewriteMetaTest(unittest.TestCase): diff --git a/tests/update.TestCase b/tests/update.TestCase index 6af32bb2..bc52981f 100755 --- a/tests/update.TestCase +++ b/tests/update.TestCase @@ -23,7 +23,6 @@ import textwrap from binascii import hexlify from datetime import datetime from pathlib import Path -from testcommon import TmpCwd, mkdtemp from unittest import mock try: @@ -61,6 +60,7 @@ import fdroidserver.metadata import fdroidserver.update from fdroidserver.common import CATEGORIES_CONFIG_NAME from fdroidserver.looseversion import LooseVersion +from testcommon import TmpCwd, mkdtemp DONATION_FIELDS = ('Donate', 'Liberapay', 'OpenCollective') From 18f3acc32e7d9d732d6186827ce3acb3d7b734cd Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 8 May 2024 16:26:46 +0200 Subject: [PATCH 7/9] split out options from read_config() There is no longer any reason for these to be intertwined. This deliberately avoids touching some files as much as possible because they are super tangled and due to be replaced. Those files are: * fdroidserver/build.py * fdroidserver/update.py # Conflicts: # tests/testcommon.py # Conflicts: # fdroidserver/btlog.py # fdroidserver/import_subcommand.py --- examples/fdroid_clean_repos.py | 5 +- .../fdroid_export_keystore_to_nitrokey.py | 4 +- examples/fdroid_exportkeystore.py | 4 +- examples/fdroid_extract_repo_pubkey.py | 4 +- examples/fdroid_fetchsrclibs.py | 5 +- examples/fdroid_nitrokeyimport.py | 4 +- fdroidserver/btlog.py | 8 +- fdroidserver/build.py | 4 +- fdroidserver/checkupdates.py | 15 ++- fdroidserver/common.py | 42 ++++++-- fdroidserver/deploy.py | 21 ++-- fdroidserver/gpgsign.py | 7 +- fdroidserver/import_subcommand.py | 15 +-- fdroidserver/init.py | 7 +- fdroidserver/install.py | 7 +- fdroidserver/lint.py | 9 +- fdroidserver/mirror.py | 28 +++--- fdroidserver/nightly.py | 5 +- fdroidserver/publish.py | 7 +- fdroidserver/readmeta.py | 4 +- fdroidserver/rewritemeta.py | 7 +- fdroidserver/scanner.py | 15 ++- fdroidserver/signatures.py | 7 +- fdroidserver/signindex.py | 7 +- fdroidserver/update.py | 4 +- fdroidserver/verify.py | 7 +- tests/build.TestCase | 6 +- tests/checkupdates.TestCase | 35 ++----- tests/common.TestCase | 98 ++++++++++++------- tests/deploy.TestCase | 40 ++++---- tests/dump_internal_metadata_format.py | 4 +- tests/exception.TestCase | 4 +- tests/gpgsign.TestCase | 4 +- tests/import_subcommand.TestCase | 10 +- tests/index.TestCase | 9 +- tests/init.TestCase | 8 +- tests/install.TestCase | 4 +- tests/key-tricks.py | 2 +- tests/lint.TestCase | 5 +- tests/main.TestCase | 2 +- tests/metadata.TestCase | 4 +- tests/net.TestCase | 2 +- tests/nightly.TestCase | 2 +- tests/parse-fdroiddata-mirror-config.py | 13 +++ tests/publish.TestCase | 10 +- tests/rewritemeta.TestCase | 2 +- tests/scanner.TestCase | 32 +++--- tests/signatures.TestCase | 4 +- tests/signindex.TestCase | 4 +- tests/testcommon.py | 14 +++ tests/update.TestCase | 6 +- tests/vcs.TestCase | 4 +- tests/verify.TestCase | 2 +- 53 files changed, 317 insertions(+), 265 deletions(-) create mode 100644 tests/parse-fdroiddata-mirror-config.py diff --git a/examples/fdroid_clean_repos.py b/examples/fdroid_clean_repos.py index aa535fc4..aaedf98a 100644 --- a/examples/fdroid_clean_repos.py +++ b/examples/fdroid_clean_repos.py @@ -23,12 +23,11 @@ def main(): help=_("applicationId with optional versionCode in the form APPID[:VERCODE]"), ) metadata.add_metadata_arguments(parser) - options = parser.parse_args() - common.options = options + options = common.parse_args(parser) pkgs = common.read_pkg_args(options.appid, True) allapps = metadata.read_metadata(pkgs) apps = common.read_app_args(options.appid, allapps, True) - common.read_config(options) + common.read_config() for appid, app in apps.items(): if "Builds" in app and len(app["Builds"]) > 0: diff --git a/examples/fdroid_export_keystore_to_nitrokey.py b/examples/fdroid_export_keystore_to_nitrokey.py index 92de2b30..8fa81ffe 100644 --- a/examples/fdroid_export_keystore_to_nitrokey.py +++ b/examples/fdroid_export_keystore_to_nitrokey.py @@ -25,8 +25,8 @@ def main(): global config parser = ArgumentParser() common.setup_global_opts(parser) - options = parser.parse_args() - config = common.read_config(options) + common.parse_args(parser) + config = common.read_config() destkeystore = config['keystore'].replace('.jks', '.p12').replace('/', '_') exportkeystore = config['keystore'].replace('.jks', '.pem').replace('/', '_') if os.path.exists(destkeystore) or os.path.exists(exportkeystore): diff --git a/examples/fdroid_exportkeystore.py b/examples/fdroid_exportkeystore.py index 9e54e397..435874a5 100644 --- a/examples/fdroid_exportkeystore.py +++ b/examples/fdroid_exportkeystore.py @@ -14,8 +14,8 @@ fdroid_summary = 'export the keystore in standard PEM format' def main(): parser = ArgumentParser() common.setup_global_opts(parser) - options = parser.parse_args() - config = common.read_config(options) + common.parse_args(parser) + config = common.read_config() env_vars = {'LC_ALL': 'C.UTF-8', 'FDROID_KEY_STORE_PASS': config['keystorepass'], 'FDROID_KEY_PASS': config['keypass']} diff --git a/examples/fdroid_extract_repo_pubkey.py b/examples/fdroid_extract_repo_pubkey.py index de13d267..f3c51767 100644 --- a/examples/fdroid_extract_repo_pubkey.py +++ b/examples/fdroid_extract_repo_pubkey.py @@ -12,8 +12,8 @@ fdroid_summary = 'export the keystore in standard PEM format' def main(): parser = ArgumentParser() common.setup_global_opts(parser) - options = parser.parse_args() - common.config = common.read_config(options) + common.parse_args(parser) + common.read_config() pubkey, repo_pubkey_fingerprint = index.extract_pubkey() print('repo_pubkey = "%s"' % pubkey.decode()) diff --git a/examples/fdroid_fetchsrclibs.py b/examples/fdroid_fetchsrclibs.py index 978baacf..299e802b 100644 --- a/examples/fdroid_fetchsrclibs.py +++ b/examples/fdroid_fetchsrclibs.py @@ -18,12 +18,11 @@ def main(): common.setup_global_opts(parser) parser.add_argument("appid", nargs='*', help=_("applicationId with optional versionCode in the form APPID[:VERCODE]")) metadata.add_metadata_arguments(parser) - options = parser.parse_args() - common.options = options + options = common.parse_args(parser) pkgs = common.read_pkg_args(options.appid, True) allapps = metadata.read_metadata(pkgs) apps = common.read_app_args(options.appid, allapps, True) - common.read_config(options) + common.read_config() srclib_dir = os.path.join('build', 'srclib') os.makedirs(srclib_dir, exist_ok=True) srclibpaths = [] diff --git a/examples/fdroid_nitrokeyimport.py b/examples/fdroid_nitrokeyimport.py index 44ec299c..9b458103 100644 --- a/examples/fdroid_nitrokeyimport.py +++ b/examples/fdroid_nitrokeyimport.py @@ -11,8 +11,8 @@ fdroid_summary = 'import the local keystore into a SmartCard HSM' def main(): parser = ArgumentParser() common.setup_global_opts(parser) - options = parser.parse_args() - config = common.read_config(options) + common.parse_args(parser) + config = common.read_config() env_vars = { 'LC_ALL': 'C.UTF-8', 'FDROID_KEY_STORE_PASS': config['keystorepass'], diff --git a/fdroidserver/btlog.py b/fdroidserver/btlog.py index 3154b3de..df889396 100755 --- a/fdroidserver/btlog.py +++ b/fdroidserver/btlog.py @@ -47,9 +47,6 @@ from . import deploy from .exception import FDroidException -options = None - - def make_binary_transparency_log( repodirs: collections.abc.Iterable, btrepo: str = 'binary_transparency', @@ -175,9 +172,8 @@ def main(): ------ :exc:`~fdroidserver.exception.FDroidException` If the specified or default Git repository does not exist. - """ - global options + """ parser = ArgumentParser() common.setup_global_opts(parser) parser.add_argument( @@ -196,7 +192,7 @@ def main(): default=None, help=_("Push the log to this git remote repository"), ) - options = parser.parse_args() + options = common.parse_args(parser) if options.verbose: logging.getLogger("requests").setLevel(logging.INFO) diff --git a/fdroidserver/build.py b/fdroidserver/build.py index 02e52315..a1fc2ff3 100644 --- a/fdroidserver/build.py +++ b/fdroidserver/build.py @@ -1014,7 +1014,7 @@ def parse_commandline(): parser.add_argument("-w", "--wiki", default=False, action="store_true", help=argparse.SUPPRESS) metadata.add_metadata_arguments(parser) - options = parser.parse_args() + options = common.parse_args(parser) metadata.warnings_action = options.W # Force --stop with --on-server to get correct exit code @@ -1076,7 +1076,7 @@ def main(): if not options.appid and not options.all: parser.error("option %s: If you really want to build all the apps, use --all" % "all") - config = common.read_config(opts=options) + config = common.read_config() if config['build_server_always']: options.server = True diff --git a/fdroidserver/checkupdates.py b/fdroidserver/checkupdates.py index 07a4d668..358a6e70 100644 --- a/fdroidserver/checkupdates.py +++ b/fdroidserver/checkupdates.py @@ -506,7 +506,7 @@ def operate_vercode(operation: str, vercode: int) -> int: return vercode -def checkupdates_app(app: metadata.App) -> None: +def checkupdates_app(app: metadata.App, auto: bool, commit: bool = False) -> None: """Check for new versions and updated name of a single app. Also write back changes to the metadata file and create a Git commit if @@ -582,7 +582,7 @@ def checkupdates_app(app: metadata.App) -> None: logging.info('...updating to version %s' % ver) commitmsg = 'Update CurrentVersion of %s to %s' % (name, ver) - if options.auto: + if auto: mode = app.AutoUpdateMode if not app.CurrentVersionCode: raise MetaDataException( @@ -665,7 +665,7 @@ def checkupdates_app(app: metadata.App) -> None: if commitmsg: metadata.write_metadata(app.metadatapath, app) - if options.commit: + if commit: logging.info("Commiting update for " + app.metadatapath) gitcmd = ["git", "commit", "-m", commitmsg] if 'auto_author' in config: @@ -695,7 +695,6 @@ def status_update_json(processed: list, failed: dict) -> None: config = None -options = None start_timestamp = time.gmtime() @@ -705,7 +704,7 @@ def main(): The behaviour of this function is influenced by the configuration file as well as command line parameters. """ - global config, options + global config # Parse command line... parser = ArgumentParser() @@ -720,10 +719,10 @@ def main(): parser.add_argument("--allow-dirty", action="store_true", default=False, help=_("Run on git repo that has uncommitted changes")) metadata.add_metadata_arguments(parser) - options = parser.parse_args() + options = common.parse_args(parser) metadata.warnings_action = options.W - config = common.read_config(options) + config = common.read_config() if not options.allow_dirty: status = subprocess.check_output(['git', 'status', '--porcelain']) @@ -749,7 +748,7 @@ def main(): logging.info(msg) try: - checkupdates_app(app) + checkupdates_app(app, options.auto, options.commit) processed.append(appid) except Exception as e: msg = _("...checkupdate failed for {appid} : {error}").format(appid=appid, error=e) diff --git a/fdroidserver/common.py b/fdroidserver/common.py index 4074d2aa..5a546386 100644 --- a/fdroidserver/common.py +++ b/fdroidserver/common.py @@ -191,6 +191,34 @@ default_config = { } +def get_options(): + """Return options as set up by parse_args(). + + This provides an easy way to get the global instance without + having to think about very confusing import and submodule + visibility. The code should be probably refactored so it does not + need this. If each individual option value was always passed to + functions as args, for example. + + https://docs.python.org/3/reference/import.html#submodules + + """ + return fdroidserver.common.options + + +def parse_args(parser): + """Call parser.parse_args(), store result in module-level variable and return it. + + This is needed to set up the copy of the options instance in the + fdroidserver.common module. A subcommand only needs to call this + if it uses functions from fdroidserver.common that expect the + "options" variable to be initialized. + + """ + fdroidserver.common.options = parser.parse_args() + return fdroidserver.common.options + + def setup_global_opts(parser): try: # the buildserver VM might not have PIL installed from PIL import PngImagePlugin @@ -373,7 +401,7 @@ def fill_config_defaults(thisconfig): thisconfig['gradle_version_dir'] = str(Path(thisconfig['cachedir']) / 'gradle') -def get_config(opts=None): +def get_config(): """Get the initalized, singleton config instance. config and options are intertwined in read_config(), so they have @@ -395,18 +423,16 @@ def get_config(opts=None): instance in fdroidserver.common. """ - global config, options + global config if config is not None: return config - read_config(opts=opts) + read_config() # make sure these values are available in common.py even if they didn't # declare global in a scope common.config = config - if opts is not None: - common.options = opts return config @@ -439,7 +465,7 @@ def config_type_check(path, data): ) -def read_config(opts=None): +def read_config(): """Read the repository config. The config is read from config_file, which is in the current @@ -458,13 +484,11 @@ def read_config(opts=None): in git, it makes sense to use a globally standard encoding. """ - global config, options + global config if config is not None: return config - options = opts - config = {} config_file = 'config.yml' old_config_file = 'config.py' diff --git a/fdroidserver/deploy.py b/fdroidserver/deploy.py index 1e28961d..b4b4cd95 100644 --- a/fdroidserver/deploy.py +++ b/fdroidserver/deploy.py @@ -36,7 +36,6 @@ from . import index from .exception import FDroidException config = None -options = None start_timestamp = time.gmtime() GIT_BRANCH = 'master' @@ -148,9 +147,10 @@ def update_awsbucket_s3cmd(repo_section): raise FDroidException() s3cmd_sync = s3cmd + ['sync', '--acl-public'] - if options.verbose: + options = common.get_options() + if options and options.verbose: s3cmd_sync += ['--verbose'] - if options.quiet: + if options and options.quiet: s3cmd_sync += ['--quiet'] s3url = s3bucketurl + '/fdroid/' @@ -312,6 +312,7 @@ def update_serverwebroot(serverwebroot, repo_section): _('rsync is missing or broken: {error}').format(error=e) ) from e rsyncargs = ['rsync', '--archive', '--delete-after', '--safe-links'] + options = common.get_options() if not options or not options.no_checksum: rsyncargs.append('--checksum') if options and options.verbose: @@ -387,7 +388,7 @@ def sync_from_localcopy(repo_section, local_copy_dir): # trailing slashes have a meaning in rsync which is not needed here, so # make sure both paths have exactly one trailing slash common.local_rsync( - options, + common.get_options(), os.path.join(local_copy_dir, repo_section).rstrip('/') + '/', repo_section.rstrip('/') + '/', ) @@ -407,7 +408,7 @@ def update_localcopy(repo_section, local_copy_dir): """ # local_copy_dir is guaranteed to have a trailing slash in main() below - common.local_rsync(options, repo_section, local_copy_dir) + common.local_rsync(common.get_options(), repo_section, local_copy_dir) offline_copy = os.path.join(os.getcwd(), BINARY_TRANSPARENCY_DIR) if os.path.isdir(os.path.join(offline_copy, '.git')): @@ -446,6 +447,8 @@ def update_servergitmirrors(servergitmirrors, repo_section): ) return + options = common.get_options() + # right now we support only 'repo' git-mirroring if repo_section == 'repo': git_mirror_path = 'git-mirror' @@ -595,7 +598,7 @@ def upload_to_android_observatory(repo_section): requests # stop unused import warning - if options.verbose: + if common.get_options().verbose: logging.getLogger("requests").setLevel(logging.INFO) logging.getLogger("urllib3").setLevel(logging.INFO) else: @@ -849,7 +852,7 @@ def push_binary_transparency(git_repo_path, git_remote): def main(): - global config, options + global config parser = ArgumentParser() common.setup_global_opts(parser) @@ -876,8 +879,8 @@ def main(): default=False, help=_("If a git mirror gets to big, allow the archive to be deleted"), ) - options = parser.parse_args() - config = common.read_config(options) + options = common.parse_args(parser) + config = common.read_config() if config.get('nonstandardwebroot') is True: standardwebroot = False diff --git a/fdroidserver/gpgsign.py b/fdroidserver/gpgsign.py index da845d4f..4ba6ebd5 100644 --- a/fdroidserver/gpgsign.py +++ b/fdroidserver/gpgsign.py @@ -28,7 +28,6 @@ from .common import FDroidPopen from .exception import FDroidException config = None -options = None start_timestamp = time.gmtime() @@ -42,14 +41,14 @@ def status_update_json(signed): def main(): - global config, options + global config # Parse command line... parser = ArgumentParser() common.setup_global_opts(parser) - options = parser.parse_args() + common.parse_args(parser) - config = common.read_config(options) + config = common.read_config() repodirs = ['repo'] if config['archive_older'] != 0: diff --git a/fdroidserver/import_subcommand.py b/fdroidserver/import_subcommand.py index d1105914..3902250e 100644 --- a/fdroidserver/import_subcommand.py +++ b/fdroidserver/import_subcommand.py @@ -45,7 +45,6 @@ from .exception import FDroidException config = None -options = None def handle_retree_error_on_windows(function, path, excinfo): @@ -55,7 +54,7 @@ def handle_retree_error_on_windows(function, path, excinfo): function(path) -def clone_to_tmp_dir(app: metadata.App) -> Path: +def clone_to_tmp_dir(app: metadata.App, rev=None) -> Path: """Clone the source repository of an app to a temporary directory for further processing. Parameters @@ -67,6 +66,7 @@ def clone_to_tmp_dir(app: metadata.App) -> Path: ------- tmp_dir The (temporary) directory the apps source has been cloned into. + """ tmp_dir = Path('tmp') tmp_dir.mkdir(exist_ok=True) @@ -76,7 +76,7 @@ def clone_to_tmp_dir(app: metadata.App) -> Path: if tmp_dir.exists(): shutil.rmtree(str(tmp_dir), onerror=handle_retree_error_on_windows) vcs = common.getvcs(app.RepoType, app.Repo, tmp_dir) - vcs.gotorevision(options.rev) + vcs.gotorevision(rev) return tmp_dir @@ -236,8 +236,9 @@ def main(): the current directory is not a Git repository, no application ID could be found, no Gradle project could be found or there is already metadata for the found application ID. + """ - global config, options + global config # Parse command line... parser = ArgumentParser() @@ -255,10 +256,10 @@ def main(): parser.add_argument("--rev", default=None, help=_("Allows a different revision (or git branch) to be specified for the initial import")) metadata.add_metadata_arguments(parser) - options = parser.parse_args() + options = common.parse_args(parser) metadata.warnings_action = options.W - config = common.read_config(options) + config = common.read_config() apps = metadata.read_metadata() app = None @@ -289,7 +290,7 @@ def main(): write_local_file = True elif options.url: app = get_app_from_url(options.url) - tmp_importer_dir = clone_to_tmp_dir(app) + tmp_importer_dir = clone_to_tmp_dir(app, options.rev) git_repo = git.Repo(tmp_importer_dir) if not options.omit_disable: diff --git a/fdroidserver/init.py b/fdroidserver/init.py index ab5a3e48..e27a7092 100644 --- a/fdroidserver/init.py +++ b/fdroidserver/init.py @@ -32,7 +32,6 @@ from . import common from .exception import FDroidException config = {} -options = None def disable_in_config(key, value): @@ -49,7 +48,7 @@ def disable_in_config(key, value): def main(): - global options, config + global config # Parse command line... parser = ArgumentParser() @@ -81,7 +80,7 @@ def main(): default=False, help=_("Do not prompt for Android SDK path, just fail"), ) - options = parser.parse_args() + options = common.parse_args(parser) common.set_console_logging(options.verbose) @@ -171,7 +170,7 @@ def main(): raise FDroidException('Repository already exists.') # now that we have a local config.yml, read configuration... - config = common.read_config(options) + config = common.read_config() # the NDK is optional and there may be multiple versions of it, so it's # left for the user to configure diff --git a/fdroidserver/install.py b/fdroidserver/install.py index 9ef82da0..b9370ee5 100644 --- a/fdroidserver/install.py +++ b/fdroidserver/install.py @@ -28,7 +28,6 @@ from . import common from .common import SdkToolsPopen from .exception import FDroidException -options = None config = None @@ -44,7 +43,7 @@ def devices(): def main(): - global options, config + global config # Parse command line... parser = ArgumentParser( @@ -63,7 +62,7 @@ def main(): default=False, help=_("Install all signed applications available"), ) - options = parser.parse_args() + options = common.parse_args(parser) common.set_console_logging(options.verbose) @@ -73,7 +72,7 @@ def main(): % "all" ) - config = common.read_config(options) + config = common.read_config() output_dir = 'repo' if not os.path.isdir(output_dir): diff --git a/fdroidserver/lint.py b/fdroidserver/lint.py index d7a9783d..1ae42852 100644 --- a/fdroidserver/lint.py +++ b/fdroidserver/lint.py @@ -31,7 +31,6 @@ from . import metadata from . import rewritemeta config = None -options = None def enforce_https(domain): @@ -503,7 +502,7 @@ def check_files_dir(app): def check_format(app): - if options.format and not rewritemeta.proper_format(app): + if common.options.format and not rewritemeta.proper_format(app): yield _("Run rewritemeta to fix formatting") @@ -787,7 +786,7 @@ def lint_config(arg): def main(): - global config, options + global config # Parse command line... parser = ArgumentParser() @@ -812,10 +811,10 @@ def main(): "appid", nargs='*', help=_("application ID of file to operate on") ) metadata.add_metadata_arguments(parser) - options = parser.parse_args() + options = common.parse_args(parser) metadata.warnings_action = options.W - config = common.read_config(options) + config = common.read_config() load_antiFeatures_config() load_categories_config() diff --git a/fdroidserver/mirror.py b/fdroidserver/mirror.py index 89e5ed66..1483ddf1 100644 --- a/fdroidserver/mirror.py +++ b/fdroidserver/mirror.py @@ -15,11 +15,9 @@ from . import common from . import index from . import update -options = None - -def _run_wget(path, urls): - if options.verbose: +def _run_wget(path, urls, verbose=False): + if verbose: verbose = '--verbose' else: verbose = '--no-verbose' @@ -48,8 +46,6 @@ def _run_wget(path, urls): def main(): - global options - parser = ArgumentParser() common.setup_global_opts(parser) parser.add_argument( @@ -93,7 +89,7 @@ def main(): parser.add_argument( "--output-dir", default=None, help=_("The directory to write the mirror to") ) - options = parser.parse_args() + options = common.parse_args(parser) common.set_console_logging(options.verbose) @@ -119,7 +115,7 @@ def main(): ) if fingerprint: - config = common.read_config(options) + config = common.read_config() if not ('jarsigner' in config or 'apksigner' in config): logging.error( _('Java JDK not found! Install in standard location or set java_paths!') @@ -229,7 +225,7 @@ def main(): _append_to_url_path(section, f[:-4] + '.log.gz') ) - _run_wget(sectiondir, urls) + _run_wget(sectiondir, urls, options.verbose) for app in data['apps']: localized = app.get('localized') @@ -242,7 +238,7 @@ def main(): if f: filepath_tuple = components + (f,) urls.append(_append_to_url_path(*filepath_tuple)) - _run_wget(os.path.join(basedir, *components), urls) + _run_wget(os.path.join(basedir, *components), urls, options.verbose) for k in update.SCREENSHOT_DIRS: urls = [] filelist = d.get(k) @@ -251,7 +247,11 @@ def main(): for f in filelist: filepath_tuple = components + (f,) urls.append(_append_to_url_path(*filepath_tuple)) - _run_wget(os.path.join(basedir, *components), urls) + _run_wget( + os.path.join(basedir, *components), + urls, + options.verbose, + ) urls = dict() for app in data['apps']: @@ -269,7 +269,11 @@ def main(): for icondir in icondirs: if icondir in urls: - _run_wget(os.path.join(basedir, section, icondir), urls[icondir]) + _run_wget( + os.path.join(basedir, section, icondir), + urls[icondir], + options.verbose, + ) if __name__ == "__main__": diff --git a/fdroidserver/nightly.py b/fdroidserver/nightly.py index 9471a26e..074a9eee 100644 --- a/fdroidserver/nightly.py +++ b/fdroidserver/nightly.py @@ -259,8 +259,7 @@ def main(): help=_("Set maximum releases in repo before older ones are archived"), ) # TODO add --with-btlog - options = parser.parse_args() - common.options = options + options = common.parse_args(parser) # force a tighter umask since this writes private key material umask = os.umask(0o077) @@ -428,7 +427,7 @@ Last updated: {date}'''.format(repo_git_base=repo_git_base, with open('config.yml', 'w') as fp: yaml.dump(config, fp, default_flow_style=False) os.chmod('config.yml', 0o600) - config = common.read_config(options) + config = common.read_config() common.assert_config_keystore(config) for root, dirs, files in os.walk(cibase): diff --git a/fdroidserver/publish.py b/fdroidserver/publish.py index b87578ad..0b499717 100644 --- a/fdroidserver/publish.py +++ b/fdroidserver/publish.py @@ -49,7 +49,6 @@ from .common import FDroidPopen from .exception import BuildException, FDroidException config = None -options = None start_timestamp = time.gmtime() @@ -269,7 +268,7 @@ def create_key_if_not_existing(keyalias): def main(): - global config, options + global config # Parse command line... parser = ArgumentParser( @@ -289,10 +288,10 @@ def main(): help=_("application ID with optional versionCode in the form APPID[:VERCODE]"), ) metadata.add_metadata_arguments(parser) - options = parser.parse_args() + options = common.parse_args(parser) metadata.warnings_action = options.W - config = common.read_config(options) + config = common.read_config() if not ('jarsigner' in config and 'keytool' in config): logging.critical( diff --git a/fdroidserver/readmeta.py b/fdroidserver/readmeta.py index e129f0c8..b8049a9f 100644 --- a/fdroidserver/readmeta.py +++ b/fdroidserver/readmeta.py @@ -20,8 +20,6 @@ from argparse import ArgumentParser from . import common from . import metadata -options = None - def main(): parser = ArgumentParser() @@ -29,7 +27,7 @@ def main(): metadata.add_metadata_arguments(parser) options = parser.parse_args() metadata.warnings_action = options.W - common.read_config(None) + common.read_config() metadata.read_metadata() diff --git a/fdroidserver/rewritemeta.py b/fdroidserver/rewritemeta.py index 5a1a6ea0..7f003ac3 100644 --- a/fdroidserver/rewritemeta.py +++ b/fdroidserver/rewritemeta.py @@ -29,7 +29,6 @@ from . import common from . import metadata config = None -options = None def proper_format(app): @@ -62,7 +61,7 @@ def remove_blank_flags_from_builds(builds): def main(): - global config, options + global config parser = ArgumentParser() common.setup_global_opts(parser) @@ -77,10 +76,10 @@ def main(): "appid", nargs='*', help=_("application ID of file to operate on") ) metadata.add_metadata_arguments(parser) - options = parser.parse_args() + options = common.parse_args(parser) metadata.warnings_action = options.W - config = common.read_config(options) + config = common.read_config() # Get all apps... allapps = metadata.read_metadata(options.appid) diff --git a/fdroidserver/scanner.py b/fdroidserver/scanner.py index 8c0a5d06..c17e7c29 100644 --- a/fdroidserver/scanner.py +++ b/fdroidserver/scanner.py @@ -40,8 +40,6 @@ from . import metadata from .exception import BuildException, VCSException, ConfigurationException from . import scanner -options = None - @dataclass class MessageStore: @@ -332,8 +330,9 @@ class ScannerTool: self.scanner_data_lookup() - config = common.get_config() - if (options and options.refresh_scanner) or config.get('refresh_scanner'): + options = common.get_options() + options_refresh_scanner = options and options.refresh_scanner + if options_refresh_scanner or common.get_config().get('refresh_scanner'): self.refresh() self.load() @@ -589,6 +588,7 @@ def scan_source(build_dir, build=metadata.Build(), json_per_build=None): ------- 0 if the problem was ignored/deleted/is only a warning, 1 otherwise """ + options = common.get_options() if toignore(path_in_build_dir): return ignoreproblem(what, path_in_build_dir, json_per_build) if todelete(path_in_build_dir): @@ -776,9 +776,6 @@ def scan_source(build_dir, build=metadata.Build(), json_per_build=None): def main(): - global options - - # Parse command line... parser = ArgumentParser( usage="%(prog)s [options] [(APPID[:VERCODE] | path/to.apk) ...]" ) @@ -793,7 +790,7 @@ def main(): parser.add_argument("-e", "--exit-code", action="store_true", default=False, help=_("Exit with a non-zero code if problems were found")) metadata.add_metadata_arguments(parser) - options = parser.parse_args() + options = common.parse_args(parser) metadata.warnings_action = options.W json_output = dict() @@ -804,7 +801,7 @@ def main(): logging.getLogger().setLevel(logging.ERROR) # initialize/load configuration values - common.get_config(opts=options) + common.get_config() probcount = 0 diff --git a/fdroidserver/signatures.py b/fdroidserver/signatures.py index b19bed8b..4f683344 100644 --- a/fdroidserver/signatures.py +++ b/fdroidserver/signatures.py @@ -103,11 +103,8 @@ def main(): "APK", nargs='*', help=_("signed APK, either a file-path or HTTPS URL.") ) parser.add_argument("--no-check-https", action="store_true", default=False) - options = parser.parse_args() - + options = common.parse_args(parser) common.set_console_logging(options.verbose) - - # Read config.py... - common.read_config(options) + common.read_config() extract(options) diff --git a/fdroidserver/signindex.py b/fdroidserver/signindex.py index ea34fd6c..4ca2d569 100644 --- a/fdroidserver/signindex.py +++ b/fdroidserver/signindex.py @@ -29,7 +29,6 @@ from . import metadata from .exception import FDroidException config = None -options = None start_timestamp = time.gmtime() @@ -175,13 +174,13 @@ def status_update_json(signed): def main(): - global config, options + global config parser = ArgumentParser() common.setup_global_opts(parser) - options = parser.parse_args() + common.parse_args(parser) - config = common.read_config(options) + config = common.read_config() if 'jarsigner' not in config: raise FDroidException( diff --git a/fdroidserver/update.py b/fdroidserver/update.py index 2ce07fa5..dac7b038 100644 --- a/fdroidserver/update.py +++ b/fdroidserver/update.py @@ -2574,10 +2574,10 @@ def main(): parser.add_argument("--allow-disabled-algorithms", action="store_true", default=False, help=_("Include APKs that are signed with disabled algorithms like MD5")) metadata.add_metadata_arguments(parser) - options = parser.parse_args() + options = common.parse_args(parser) metadata.warnings_action = options.W - config = common.read_config(options) + config = common.read_config() common.setup_status_output(start_timestamp) if not (('jarsigner' in config or 'apksigner' in config) diff --git a/fdroidserver/verify.py b/fdroidserver/verify.py index 765f23c9..41b46ada 100644 --- a/fdroidserver/verify.py +++ b/fdroidserver/verify.py @@ -30,7 +30,6 @@ from . import common from . import net from .exception import FDroidException -options = None config = None @@ -146,7 +145,7 @@ def write_json_report(url, remote_apk, unsigned_apk, compare_result): def main(): - global options, config + global config # Parse command line... parser = ArgumentParser( @@ -170,9 +169,9 @@ def main(): default=False, help=_("Output JSON report to file named after APK."), ) - options = parser.parse_args() + options = common.parse_args(parser) - config = common.read_config(options) + config = common.read_config() tmp_dir = 'tmp' if not os.path.isdir(tmp_dir): diff --git a/tests/build.TestCase b/tests/build.TestCase index 0228bf35..f6607634 100755 --- a/tests/build.TestCase +++ b/tests/build.TestCase @@ -28,7 +28,7 @@ import fdroidserver.common import fdroidserver.metadata import fdroidserver.scanner import fdroidserver.vmtools -from testcommon import mkdtemp +from testcommon import mkdtemp, parse_args_for_test class FakeProcess: @@ -561,7 +561,7 @@ class BuildTest(unittest.TestCase): os.chdir(self.testdir) os.mkdir("build") - config = fdroidserver.common.get_config() + config = fdroidserver.common.read_config() config['sdk_path'] = os.getenv('ANDROID_HOME') config['ndk_paths'] = {'r10d': os.getenv('ANDROID_NDK_HOME')} fdroidserver.common.config = config @@ -1113,7 +1113,7 @@ if __name__ == "__main__": default=False, help="Spew out even more information than normal", ) - fdroidserver.common.options = parser.parse_args(['--verbose']) + parse_args_for_test(parser, sys.argv) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(BuildTest)) diff --git a/tests/checkupdates.TestCase b/tests/checkupdates.TestCase index 0ae895de..9ff7eb98 100755 --- a/tests/checkupdates.TestCase +++ b/tests/checkupdates.TestCase @@ -29,8 +29,6 @@ class CheckupdatesTest(unittest.TestCase): os.chdir(self.basedir) def test_autoupdatemode_no_suffix(self): - fdroidserver.checkupdates.options = mock.Mock() - fdroidserver.checkupdates.options.auto = 'bleh' fdroidserver.checkupdates.config = {} app = fdroidserver.metadata.App() @@ -51,7 +49,7 @@ class CheckupdatesTest(unittest.TestCase): ): with mock.patch('fdroidserver.metadata.write_metadata', mock.Mock()): with mock.patch('subprocess.call', lambda cmd: 0): - fdroidserver.checkupdates.checkupdates_app(app) + fdroidserver.checkupdates.checkupdates_app(app, auto=True) build = app['Builds'][-1] self.assertEqual(build.versionName, '1.1.9') @@ -63,15 +61,13 @@ class CheckupdatesTest(unittest.TestCase): with mock.patch('fdroidserver.metadata.write_metadata', mock.Mock()): with mock.patch('subprocess.call', lambda cmd: 0): with self.assertRaises(FDroidException): - fdroidserver.checkupdates.checkupdates_app(app) + fdroidserver.checkupdates.checkupdates_app(app, auto=True) build = app['Builds'][-1] self.assertEqual(build.versionName, '1.1.9') self.assertEqual(build.commit, '1.1.9') def test_autoupdatemode_suffix(self): - fdroidserver.checkupdates.options = mock.Mock() - fdroidserver.checkupdates.options.auto = 'bleh' fdroidserver.checkupdates.config = {} app = fdroidserver.metadata.App() @@ -92,15 +88,13 @@ class CheckupdatesTest(unittest.TestCase): ): with mock.patch('fdroidserver.metadata.write_metadata', mock.Mock()): with mock.patch('subprocess.call', lambda cmd: 0): - fdroidserver.checkupdates.checkupdates_app(app) + fdroidserver.checkupdates.checkupdates_app(app, auto=True) build = app['Builds'][-1] self.assertEqual(build.versionName, '1.1.9.10109-fdroid') self.assertEqual(build.commit, 'v1.1.9_10109') def test_autoupdate_multi_variants(self): - fdroidserver.checkupdates.options = mock.Mock() - fdroidserver.checkupdates.options.auto = 'bleh' fdroidserver.checkupdates.config = {} app = fdroidserver.metadata.App() @@ -133,7 +127,7 @@ class CheckupdatesTest(unittest.TestCase): ): with mock.patch('fdroidserver.metadata.write_metadata', mock.Mock()): with mock.patch('subprocess.call', lambda cmd: 0): - fdroidserver.checkupdates.checkupdates_app(app) + fdroidserver.checkupdates.checkupdates_app(app, auto=True) build = app['Builds'][-2] self.assertEqual(build.versionName, '1.1.9') @@ -149,8 +143,6 @@ class CheckupdatesTest(unittest.TestCase): self.assertEqual(app.CurrentVersionCode, 101093) def test_checkupdates_app_http(self): - fdroidserver.checkupdates.options = mock.Mock() - fdroidserver.checkupdates.options.auto = 'bleh' fdroidserver.checkupdates.config = {} app = fdroidserver.metadata.App() @@ -164,7 +156,7 @@ class CheckupdatesTest(unittest.TestCase): 'fdroidserver.checkupdates.check_http', lambda app: (None, 'bla') ): with self.assertRaises(FDroidException): - fdroidserver.checkupdates.checkupdates_app(app) + fdroidserver.checkupdates.checkupdates_app(app, auto=True) with mock.patch( 'fdroidserver.checkupdates.check_http', lambda app: ('1.1.9', 10109) @@ -173,12 +165,10 @@ class CheckupdatesTest(unittest.TestCase): 'fdroidserver.metadata.write_metadata', mock.Mock() ) as wrmock: with mock.patch('subprocess.call', lambda cmd: 0): - fdroidserver.checkupdates.checkupdates_app(app) + fdroidserver.checkupdates.checkupdates_app(app, auto=True) wrmock.assert_called_with(app.metadatapath, app) def test_checkupdates_app_tags(self): - fdroidserver.checkupdates.options = mock.Mock() - fdroidserver.checkupdates.options.auto = 'bleh' fdroidserver.checkupdates.config = {} app = fdroidserver.metadata.App() @@ -199,7 +189,7 @@ class CheckupdatesTest(unittest.TestCase): lambda app, pattern: (None, 'bla', None), ): with self.assertRaises(FDroidException): - fdroidserver.checkupdates.checkupdates_app(app) + fdroidserver.checkupdates.checkupdates_app(app, auto=True) with mock.patch( 'fdroidserver.checkupdates.check_tags', @@ -207,15 +197,13 @@ class CheckupdatesTest(unittest.TestCase): ): with mock.patch('fdroidserver.metadata.write_metadata', mock.Mock()): with mock.patch('subprocess.call', lambda cmd: 0): - fdroidserver.checkupdates.checkupdates_app(app) + fdroidserver.checkupdates.checkupdates_app(app, auto=True) build = app['Builds'][-1] self.assertEqual(build.versionName, '1.1.9') self.assertEqual(build.commit, 'v1.1.9') def test_check_http(self): - fdroidserver.checkupdates.options = mock.Mock() - app = fdroidserver.metadata.App() app.id = 'loop.starts.shooting' app.metadatapath = 'metadata/' + app.id + '.yml' @@ -242,8 +230,6 @@ class CheckupdatesTest(unittest.TestCase): fdroidserver.checkupdates.check_http(app) def test_check_http_ignore(self): - fdroidserver.checkupdates.options = mock.Mock() - app = fdroidserver.metadata.App() app.id = 'loop.starts.shooting' app.metadatapath = 'metadata/' + app.id + '.yml' @@ -259,8 +245,6 @@ class CheckupdatesTest(unittest.TestCase): self.assertEqual(vername, None) def test_check_tags_data(self): - fdroidserver.checkupdates.options = mock.Mock() - app = fdroidserver.metadata.App() app.id = 'loop.starts.shooting' app.metadatapath = 'metadata/' + app.id + '.yml' @@ -336,6 +320,7 @@ class CheckupdatesTest(unittest.TestCase): if __name__ == "__main__": import argparse + from testcommon import parse_args_for_test parser = argparse.ArgumentParser() parser.add_argument( @@ -345,7 +330,7 @@ if __name__ == "__main__": default=False, help="Spew out even more information than normal", ) - fdroidserver.common.options = parser.parse_args(['--verbose']) + parse_args_for_test(parser, sys.argv) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(CheckupdatesTest)) diff --git a/tests/common.TestCase b/tests/common.TestCase index b3e32100..5d3b5cdb 100755 --- a/tests/common.TestCase +++ b/tests/common.TestCase @@ -38,7 +38,7 @@ import fdroidserver.index import fdroidserver.signindex import fdroidserver.common import fdroidserver.metadata -from testcommon import TmpCwd, mkdtemp +from testcommon import TmpCwd, mkdtemp, parse_args_for_test from fdroidserver.common import ANTIFEATURES_CONFIG_NAME, CATEGORIES_CONFIG_NAME from fdroidserver.exception import FDroidException, VCSException,\ MetaDataException, VerificationException @@ -491,7 +491,7 @@ class CommonTest(unittest.TestCase): def test_signjar(self): _mock_common_module_options_instance() - config = fdroidserver.common.read_config(fdroidserver.common.options) + config = fdroidserver.common.read_config() config['jarsigner'] = fdroidserver.common.find_sdk_tools_cmd('jarsigner') fdroidserver.common.config = config fdroidserver.signindex.config = config @@ -512,7 +512,7 @@ class CommonTest(unittest.TestCase): def test_verify_apk_signature(self): _mock_common_module_options_instance() - config = fdroidserver.common.read_config(fdroidserver.common.options) + config = fdroidserver.common.read_config() fdroidserver.common.config = config self.assertTrue(fdroidserver.common.verify_apk_signature('bad-unicode-πÇÇ现代通用字-български-عربي1.apk')) @@ -535,7 +535,7 @@ class CommonTest(unittest.TestCase): def test_verify_old_apk_signature(self): _mock_common_module_options_instance() - config = fdroidserver.common.read_config(fdroidserver.common.options) + config = fdroidserver.common.read_config() config['jarsigner'] = fdroidserver.common.find_sdk_tools_cmd('jarsigner') fdroidserver.common.config = config @@ -556,7 +556,7 @@ class CommonTest(unittest.TestCase): def test_verify_jar_signature(self): """Sign entry.jar and make sure it validates""" - config = fdroidserver.common.read_config(fdroidserver.common.options) + config = fdroidserver.common.read_config() config['jarsigner'] = fdroidserver.common.find_sdk_tools_cmd('jarsigner') config['keystore'] = os.path.join(self.basedir, 'keystore.jks') config['repo_keyalias'] = 'sova' @@ -574,7 +574,7 @@ class CommonTest(unittest.TestCase): def test_verify_jar_signature_fails(self): """Test verify_jar_signature fails on unsigned and deprecated algorithms""" - config = fdroidserver.common.read_config(fdroidserver.common.options) + config = fdroidserver.common.read_config() config['jarsigner'] = fdroidserver.common.find_sdk_tools_cmd('jarsigner') fdroidserver.common.config = config source_dir = os.path.join(self.basedir, 'signindex') @@ -584,7 +584,7 @@ class CommonTest(unittest.TestCase): fdroidserver.common.verify_jar_signature(testfile) def test_verify_deprecated_jar_signature(self): - config = fdroidserver.common.read_config(fdroidserver.common.options) + config = fdroidserver.common.read_config() config['jarsigner'] = fdroidserver.common.find_sdk_tools_cmd('jarsigner') fdroidserver.common.config = config source_dir = os.path.join(self.basedir, 'signindex') @@ -597,7 +597,7 @@ class CommonTest(unittest.TestCase): fdroidserver.common.verify_deprecated_jar_signature(testfile) def test_verify_apks(self): - config = fdroidserver.common.read_config(fdroidserver.common.options) + config = fdroidserver.common.read_config() fdroidserver.common.config = config _mock_common_module_options_instance() @@ -907,7 +907,7 @@ class CommonTest(unittest.TestCase): def test_sign_apk(self): _mock_common_module_options_instance() - config = fdroidserver.common.read_config(fdroidserver.common.options) + config = fdroidserver.common.read_config() if 'apksigner' not in config: self.skipTest('SKIPPING test_sign_apk, apksigner not installed!') @@ -978,7 +978,7 @@ class CommonTest(unittest.TestCase): @unittest.skipIf(os.getuid() == 0, 'This is meaningless when run as root') def test_sign_apk_fail(self): _mock_common_module_options_instance() - config = fdroidserver.common.read_config(fdroidserver.common.options) + config = fdroidserver.common.read_config() if 'apksigner' not in config: self.skipTest('SKIPPING test_sign_apk_fail, apksigner not installed!') @@ -1002,7 +1002,7 @@ class CommonTest(unittest.TestCase): def test_sign_apk_corrupt(self): _mock_common_module_options_instance() - config = fdroidserver.common.read_config(fdroidserver.common.options) + config = fdroidserver.common.read_config() if 'apksigner' not in config: self.skipTest('SKIPPING test_sign_apk_corrupt, apksigner not installed!') @@ -1029,7 +1029,7 @@ class CommonTest(unittest.TestCase): def test_resign_apk(self): """When using apksigner, it should resign signed APKs""" _mock_common_module_options_instance() - config = fdroidserver.common.read_config(fdroidserver.common.options) + config = fdroidserver.common.read_config() if 'apksigner' not in config: self.skipTest('SKIPPING test_resign_apk, apksigner not installed!') @@ -1935,7 +1935,7 @@ class CommonTest(unittest.TestCase): os.chdir(self.tmpdir) self.assertFalse(os.path.exists('config.yml')) self.assertFalse(os.path.exists('config.py')) - config = fdroidserver.common.read_config(fdroidserver.common.options) + config = fdroidserver.common.read_config() self.assertFalse(config.get('update_stats')) self.assertIsNotNone(config.get('char_limits')) @@ -1945,7 +1945,7 @@ class CommonTest(unittest.TestCase): open('config.yml', 'w').close() self.assertTrue(os.path.exists('config.yml')) self.assertFalse(os.path.exists('config.py')) - config = fdroidserver.common.read_config(fdroidserver.common.options) + config = fdroidserver.common.read_config() self.assertFalse(config.get('update_stats')) self.assertIsNotNone(config.get('char_limits')) @@ -1956,7 +1956,7 @@ class CommonTest(unittest.TestCase): fp.write('apksigner: yml') self.assertTrue(os.path.exists('config.yml')) self.assertFalse(os.path.exists('config.py')) - config = fdroidserver.common.read_config(fdroidserver.common.options) + config = fdroidserver.common.read_config() self.assertEqual('yml', config.get('apksigner')) def test_with_config_yml_utf8(self): @@ -1967,7 +1967,7 @@ class CommonTest(unittest.TestCase): fp.write('apksigner: ' + teststr) self.assertTrue(os.path.exists('config.yml')) self.assertFalse(os.path.exists('config.py')) - config = fdroidserver.common.read_config(fdroidserver.common.options) + config = fdroidserver.common.read_config() self.assertEqual(teststr, config.get('apksigner')) def test_with_config_yml_utf8_as_ascii(self): @@ -1978,7 +1978,7 @@ class CommonTest(unittest.TestCase): yaml.dump({'apksigner': teststr}, fp) self.assertTrue(os.path.exists('config.yml')) self.assertFalse(os.path.exists('config.py')) - config = fdroidserver.common.read_config(fdroidserver.common.options) + config = fdroidserver.common.read_config() self.assertEqual(teststr, config.get('apksigner')) def test_with_config_yml_with_env_var(self): @@ -1990,20 +1990,20 @@ class CommonTest(unittest.TestCase): fp.write("""keypass: {'env': 'SECRET'}""") self.assertTrue(os.path.exists('config.yml')) self.assertFalse(os.path.exists('config.py')) - config = fdroidserver.common.read_config(fdroidserver.common.options) + config = fdroidserver.common.read_config() self.assertEqual(os.getenv('SECRET', 'fail'), config.get('keypass')) def test_with_config_yml_is_dict(self): os.chdir(self.tmpdir) Path('config.yml').write_text('apksigner = /placeholder/path') with self.assertRaises(TypeError): - fdroidserver.common.read_config(fdroidserver.common.options) + fdroidserver.common.read_config() def test_with_config_yml_is_not_mixed_type(self): os.chdir(self.tmpdir) Path('config.yml').write_text('k: v\napksigner = /placeholder/path') with self.assertRaises(yaml.scanner.ScannerError): - fdroidserver.common.read_config(fdroidserver.common.options) + fdroidserver.common.read_config() def test_with_config_py(self): """Make sure it is still possible to use config.py alone.""" @@ -2012,7 +2012,7 @@ class CommonTest(unittest.TestCase): fp.write('apksigner = "py"') self.assertFalse(os.path.exists('config.yml')) self.assertTrue(os.path.exists('config.py')) - config = fdroidserver.common.read_config(fdroidserver.common.options) + config = fdroidserver.common.read_config() self.assertEqual("py", config.get('apksigner')) def test_config_perm_warning(self): @@ -2022,7 +2022,7 @@ class CommonTest(unittest.TestCase): fp.write('keystore: foo.jks') self.assertTrue(os.path.exists(fp.name)) os.chmod(fp.name, 0o666) - fdroidserver.common.read_config(fdroidserver.common.options) + fdroidserver.common.read_config() os.remove(fp.name) fdroidserver.common.config = None @@ -2030,7 +2030,7 @@ class CommonTest(unittest.TestCase): fp.write('keystore = "foo.jks"') self.assertTrue(os.path.exists(fp.name)) os.chmod(fp.name, 0o666) - fdroidserver.common.read_config(fdroidserver.common.options) + fdroidserver.common.read_config() def test_with_both_config_yml_py(self): """If config.yml and config.py are present, config.py should be ignored.""" @@ -2041,7 +2041,7 @@ class CommonTest(unittest.TestCase): fp.write('apksigner = "py"') self.assertTrue(os.path.exists('config.yml')) self.assertTrue(os.path.exists('config.py')) - config = fdroidserver.common.read_config(fdroidserver.common.options) + config = fdroidserver.common.read_config() self.assertEqual('yml', config.get('apksigner')) def test_config_repo_url(self): @@ -2092,14 +2092,14 @@ class CommonTest(unittest.TestCase): fp.write('apksigner: yml') self.assertTrue(os.path.exists(fp.name)) self.assertFalse(os.path.exists('config.py')) - config = fdroidserver.common.read_config(fdroidserver.common.options) + config = fdroidserver.common.read_config() self.assertFalse('keypass' in config) self.assertEqual('yml', config.get('apksigner')) fdroidserver.common.write_to_config(config, 'keypass', 'mysecretpassword') with open(fp.name) as fp: print(fp.read()) fdroidserver.common.config = None - config = fdroidserver.common.read_config(fdroidserver.common.options) + config = fdroidserver.common.read_config() self.assertEqual('mysecretpassword', config['keypass']) def test_write_to_config_py(self): @@ -2108,12 +2108,12 @@ class CommonTest(unittest.TestCase): fp.write('apksigner = "py"') self.assertTrue(os.path.exists(fp.name)) self.assertFalse(os.path.exists('config.yml')) - config = fdroidserver.common.read_config(fdroidserver.common.options) + config = fdroidserver.common.read_config() self.assertFalse('keypass' in config) self.assertEqual('py', config.get('apksigner')) fdroidserver.common.write_to_config(config, 'keypass', 'mysecretpassword') fdroidserver.common.config = None - config = fdroidserver.common.read_config(fdroidserver.common.options) + config = fdroidserver.common.read_config() self.assertEqual('mysecretpassword', config['keypass']) def test_config_dict_with_int_keys(self): @@ -2122,7 +2122,7 @@ class CommonTest(unittest.TestCase): fp.write('java_paths:\n 8: /usr/lib/jvm/java-8-openjdk\n') self.assertTrue(os.path.exists(fp.name)) self.assertFalse(os.path.exists('config.py')) - config = fdroidserver.common.read_config(fdroidserver.common.options) + config = fdroidserver.common.read_config() self.assertEqual('/usr/lib/jvm/java-8-openjdk', config.get('java_paths', {}).get('8')) @mock.patch.dict(os.environ, {'PATH': os.getenv('PATH')}, clear=True) @@ -2200,7 +2200,7 @@ class CommonTest(unittest.TestCase): shutil.copy(os.path.join(self.basedir, '..', 'buildserver', 'config.buildserver.yml'), 'config.yml') self.assertFalse(os.path.exists('config.py')) - fdroidserver.common.read_config(fdroidserver.common.options) + fdroidserver.common.read_config() def test_setup_status_output(self): os.chdir(self.tmpdir) @@ -3267,11 +3267,43 @@ class ConfigOptionsScopeTest(unittest.TestCase): global options del options + def test_parse_args(self): + """Test that options is properly set up at the module-level and not global.""" + self.assertFalse('options' in globals()) + self.assertIsNone(fdroidserver.common.options) + parser = ArgumentParser() + fdroidserver.common.setup_global_opts(parser) + with mock.patch('sys.argv', ['$0']): + o = fdroidserver.common.parse_args(parser) + self.assertEqual(o, fdroidserver.common.options) + + # No function should set options as a global, and the global + # keyword does not create the variable. + global options + with self.assertRaises(NameError): + options + self.assertFalse('options' in globals()) + + def test_parse_args_without_args(self): + """Test that the parsing function works fine when there are no args.""" + parser = ArgumentParser() + fdroidserver.common.setup_global_opts(parser) + with mock.patch('sys.argv', ['$0']): + o = fdroidserver.common.parse_args(parser) + self.assertFalse(o.verbose) + + def test_parse_args_with_args(self): + parser = ArgumentParser() + fdroidserver.common.setup_global_opts(parser) + with mock.patch('sys.argv', ['$0', '-v']): + o = fdroidserver.common.parse_args(parser) + self.assertTrue(o.verbose) + def test_get_config(self): """Show how the module-level variables are initialized.""" self.assertTrue('config' not in vars() and 'config' not in globals()) self.assertIsNone(fdroidserver.common.config) - config = fdroidserver.common.get_config() + config = fdroidserver.common.read_config() self.assertIsNotNone(fdroidserver.common.config) self.assertEqual(dict, type(config)) self.assertEqual(config, fdroidserver.common.config) @@ -3281,7 +3313,7 @@ class ConfigOptionsScopeTest(unittest.TestCase): global config self.assertTrue('config' not in vars() and 'config' not in globals()) self.assertIsNone(fdroidserver.common.config) - c = fdroidserver.common.get_config() + c = fdroidserver.common.read_config() self.assertIsNotNone(fdroidserver.common.config) self.assertEqual(dict, type(c)) self.assertEqual(c, fdroidserver.common.config) @@ -3302,7 +3334,7 @@ if __name__ == "__main__": default=False, help="Spew out even more information than normal", ) - fdroidserver.common.options = parser.parse_args(['--verbose']) + parse_args_for_test(parser, sys.argv) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(CommonTest)) diff --git a/tests/deploy.TestCase b/tests/deploy.TestCase index f2082dd3..4a2365c9 100755 --- a/tests/deploy.TestCase +++ b/tests/deploy.TestCase @@ -18,7 +18,7 @@ if localmodule not in sys.path: import fdroidserver.common import fdroidserver.deploy from fdroidserver.exception import FDroidException -from testcommon import TmpCwd, mkdtemp +from testcommon import TmpCwd, mkdtemp, parse_args_for_test class DeployTest(unittest.TestCase): @@ -114,11 +114,11 @@ class DeployTest(unittest.TestCase): self.maxDiff = None # setup parameters for this test run - fdroidserver.deploy.options = mock.Mock() - fdroidserver.deploy.options.no_checksum = True - fdroidserver.deploy.options.identity_file = None - fdroidserver.deploy.options.verbose = False - fdroidserver.deploy.options.quiet = True + fdroidserver.common.options = mock.Mock() + fdroidserver.common.options.no_checksum = True + fdroidserver.common.options.identity_file = None + fdroidserver.common.options.verbose = False + fdroidserver.common.options.quiet = True fdroidserver.deploy.config = {'make_current_version_link': True} url = "example.com:/var/www/fdroid" repo_section = 'repo' @@ -207,12 +207,12 @@ class DeployTest(unittest.TestCase): def test_update_serverwebroot_with_id_file(self): # setup parameters for this test run - fdroidserver.deploy.options = mock.Mock() - fdroidserver.deploy.options.identity_file = None - fdroidserver.deploy.options.no_checksum = True - fdroidserver.deploy.options.verbose = True - fdroidserver.deploy.options.quiet = False - fdroidserver.deploy.options.identity_file = None + fdroidserver.common.options = mock.Mock() + fdroidserver.common.options.identity_file = None + fdroidserver.common.options.no_checksum = True + fdroidserver.common.options.verbose = True + fdroidserver.common.options.quiet = False + fdroidserver.common.options.identity_file = None fdroidserver.deploy.config = {'identity_file': './id_rsa'} url = "example.com:/var/www/fdroid" repo_section = 'archive' @@ -289,7 +289,7 @@ class DeployTest(unittest.TestCase): not os.getenv('VIRUSTOTAL_API_KEY'), 'VIRUSTOTAL_API_KEY is not set' ) def test_upload_to_virustotal(self): - fdroidserver.deploy.options.verbose = True + fdroidserver.common.options.verbose = True virustotal_apikey = os.getenv('VIRUSTOTAL_API_KEY') fdroidserver.deploy.upload_to_virustotal('repo', virustotal_apikey) @@ -307,12 +307,12 @@ class DeployTest(unittest.TestCase): def test_update_servergitmirrors(self): # setup parameters for this test run - fdroidserver.deploy.options = mock.Mock() - fdroidserver.deploy.options.identity_file = None - fdroidserver.deploy.options.no_keep_git_mirror_archive = False - fdroidserver.deploy.options.verbose = False - fdroidserver.deploy.options.quiet = True - fdroidserver.deploy.options.index_only = False + fdroidserver.common.options = mock.Mock() + fdroidserver.common.options.identity_file = None + fdroidserver.common.options.no_keep_git_mirror_archive = False + fdroidserver.common.options.verbose = False + fdroidserver.common.options.quiet = True + fdroidserver.common.options.index_only = False config = {} fdroidserver.common.fill_config_defaults(config) @@ -399,7 +399,7 @@ if __name__ == "__main__": default=False, help="Spew out even more information than normal", ) - fdroidserver.common.options = parser.parse_args(['--verbose']) + parse_args_for_test(parser, sys.argv) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(DeployTest)) diff --git a/tests/dump_internal_metadata_format.py b/tests/dump_internal_metadata_format.py index 24b7e911..f9763ebc 100755 --- a/tests/dump_internal_metadata_format.py +++ b/tests/dump_internal_metadata_format.py @@ -56,9 +56,9 @@ def _build_yaml_representer(dumper, data): parser = ArgumentParser() fdroidserver.common.setup_global_opts(parser) fdroidserver.metadata.add_metadata_arguments(parser) -options = parser.parse_args() +options = fdroidserver.common.parse_args(parser) fdroidserver.metadata.warnings_action = options.W -fdroidserver.common.read_config(None) +fdroidserver.common.read_config() if not os.path.isdir('metadata'): print("This script must be run in an F-Droid data folder with a 'metadata' subdir!") diff --git a/tests/exception.TestCase b/tests/exception.TestCase index bfa44f57..f3e69e69 100755 --- a/tests/exception.TestCase +++ b/tests/exception.TestCase @@ -57,6 +57,7 @@ if __name__ == "__main__": os.chdir(os.path.dirname(__file__)) import argparse + from testcommon import parse_args_for_test parser = argparse.ArgumentParser() parser.add_argument( @@ -66,8 +67,7 @@ if __name__ == "__main__": default=False, help="Spew out even more information than normal", ) - fdroidserver.exception.options = parser.parse_args(['--verbose']) - fdroidserver.common.options = fdroidserver.exception.options + parse_args_for_test(parser, sys.argv) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(ExceptionTest)) diff --git a/tests/gpgsign.TestCase b/tests/gpgsign.TestCase index 27feb79e..1c013c1b 100755 --- a/tests/gpgsign.TestCase +++ b/tests/gpgsign.TestCase @@ -32,7 +32,7 @@ class GpgsignTest(unittest.TestCase): self.repodir.mkdir() gpgsign.config = None - config = common.read_config(common.options) + config = common.read_config() config['verbose'] = True config['gpghome'] = str((self.basedir / 'gnupghome').resolve()) config['gpgkey'] = '1DBA2E89' @@ -84,7 +84,7 @@ if __name__ == "__main__": default=False, help="Spew out even more information than normal", ) - common.options = parser.parse_args(['--verbose']) + common.parse_args(parser) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(GpgsignTest)) diff --git a/tests/import_subcommand.TestCase b/tests/import_subcommand.TestCase index 1b3347ad..b37c2f37 100755 --- a/tests/import_subcommand.TestCase +++ b/tests/import_subcommand.TestCase @@ -24,7 +24,7 @@ import fdroidserver.common import fdroidserver.import_subcommand import fdroidserver.metadata from fdroidserver.exception import FDroidException -from testcommon import TmpCwd, mkdtemp +from testcommon import TmpCwd, mkdtemp, parse_args_for_test class ImportTest(unittest.TestCase): @@ -33,8 +33,6 @@ class ImportTest(unittest.TestCase): def setUp(self): logging.basicConfig(level=logging.DEBUG) self.basedir = localmodule / 'tests' - fdroidserver.import_subcommand.options = mock.Mock() - fdroidserver.import_subcommand.options.rev = None os.chdir(self.basedir) self._td = mkdtemp() self.testdir = self._td.name @@ -145,7 +143,9 @@ class ImportTest(unittest.TestCase): fdroidserver.import_subcommand.main() @mock.patch('sys.argv', ['fdroid import', '-u', 'https://fake/git/url.git']) - @mock.patch('fdroidserver.import_subcommand.clone_to_tmp_dir', lambda a: Path('td')) + @mock.patch( + 'fdroidserver.import_subcommand.clone_to_tmp_dir', lambda a, r: Path('td') + ) def test_main_local_git(self): os.chdir(self.testdir) git.Repo.init('td') @@ -170,7 +170,7 @@ if __name__ == "__main__": default=False, help="Spew out even more information than normal", ) - fdroidserver.common.options = parser.parse_args(['--verbose']) + parse_args_for_test(parser, sys.argv) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(ImportTest)) diff --git a/tests/index.TestCase b/tests/index.TestCase index 3e70acb1..0e23c71b 100755 --- a/tests/index.TestCase +++ b/tests/index.TestCase @@ -25,7 +25,7 @@ if localmodule not in sys.path: import fdroidserver from fdroidserver import common, index, publish, signindex, update -from testcommon import TmpCwd, mkdtemp +from testcommon import TmpCwd, mkdtemp, parse_args_for_test from pathlib import Path @@ -55,7 +55,7 @@ class IndexTest(unittest.TestCase): common.config = None common.options = Options - config = common.read_config(common.options) + config = common.read_config() config['jarsigner'] = common.find_sdk_tools_cmd('jarsigner') common.config = config signindex.config = config @@ -751,7 +751,7 @@ class IndexTest(unittest.TestCase): yaml.dump(c, fp) os.system('cat config.yml') common.config = None - common.read_config(Options) + common.read_config() repodict = {'address': common.config['repo_url']} index.add_mirrors_to_repodict('repo', repodict) self.assertEqual( @@ -951,8 +951,7 @@ if __name__ == "__main__": default=False, help="Spew out even more information than normal", ) - options = parser.parse_args(["--verbose"]) - Options.verbose = options.verbose + parse_args_for_test(parser, sys.argv) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(IndexTest)) diff --git a/tests/init.TestCase b/tests/init.TestCase index 9266fa7c..7e674cb5 100755 --- a/tests/init.TestCase +++ b/tests/init.TestCase @@ -18,7 +18,7 @@ if localmodule not in sys.path: sys.path.insert(0, localmodule) import fdroidserver.init -from testcommon import mkdtemp +from testcommon import mkdtemp, parse_args_for_test class InitTest(unittest.TestCase): @@ -42,14 +42,14 @@ class InitTest(unittest.TestCase): fp.write('keystore: NONE\n') fp.write('keypass: mysupersecrets\n') os.chmod('config.yml', 0o600) - config = fdroidserver.common.read_config(fdroidserver.common.options) + config = fdroidserver.common.read_config() self.assertEqual('NONE', config['keystore']) self.assertEqual('mysupersecrets', config['keypass']) fdroidserver.init.disable_in_config('keypass', 'comment') with open(fp.name) as fp: self.assertTrue('#keypass:' in fp.read()) fdroidserver.common.config = None - config = fdroidserver.common.read_config(fdroidserver.common.options) + config = fdroidserver.common.read_config() self.assertIsNone(config.get('keypass')) @unittest.skipIf(os.name == 'nt', "calling main() like this hangs on Windows") @@ -84,7 +84,7 @@ if __name__ == "__main__": default=False, help="Spew out even more information than normal", ) - fdroidserver.init.options = parser.parse_args(['--verbose']) + fdroidserver.init.options = parse_args_for_test(parser, sys.argv) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(InitTest)) diff --git a/tests/install.TestCase b/tests/install.TestCase index 2f4569ea..cef5c022 100755 --- a/tests/install.TestCase +++ b/tests/install.TestCase @@ -38,6 +38,7 @@ if __name__ == "__main__": os.chdir(os.path.dirname(__file__)) import argparse + from testcommon import parse_args_for_test parser = argparse.ArgumentParser() parser.add_argument( @@ -47,8 +48,7 @@ if __name__ == "__main__": default=False, help="Spew out even more information than normal", ) - fdroidserver.install.options = parser.parse_args(['--verbose']) - fdroidserver.common.options = fdroidserver.install.options + fdroidserver.install.options = parse_args_for_test(parser, sys.argv) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(InstallTest)) diff --git a/tests/key-tricks.py b/tests/key-tricks.py index b634803f..7fc0f3ea 100755 --- a/tests/key-tricks.py +++ b/tests/key-tricks.py @@ -11,7 +11,7 @@ if os.getenv('CI') is None: sys.exit(1) os.chdir(os.path.dirname(__file__)) -config = fdroidserver.common.read_config(common.options) +config = fdroidserver.common.read_config() nightly.PASSWORD = config['keystorepass'] nightly.KEY_ALIAS = config['repo_keyalias'] diff --git a/tests/lint.TestCase b/tests/lint.TestCase index d9ac8de2..5937aadf 100755 --- a/tests/lint.TestCase +++ b/tests/lint.TestCase @@ -20,7 +20,7 @@ import fdroidserver.common import fdroidserver.lint import fdroidserver.metadata from fdroidserver.common import CATEGORIES_CONFIG_NAME -from testcommon import mkdtemp +from testcommon import mkdtemp, parse_args_for_test class LintTest(unittest.TestCase): @@ -536,8 +536,7 @@ if __name__ == "__main__": default=False, help="Spew out even more information than normal", ) - fdroidserver.lint.options = parser.parse_args(['--verbose']) - fdroidserver.common.options = fdroidserver.lint.options + fdroidserver.lint.options = parse_args_for_test(parser, sys.argv) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(LintTest)) diff --git a/tests/main.TestCase b/tests/main.TestCase index 9ce25a2b..691a0aca 100755 --- a/tests/main.TestCase +++ b/tests/main.TestCase @@ -274,7 +274,7 @@ if __name__ == "__main__": default=False, help="Spew out even more information than normal", ) - common.options = parser.parse_args(['--verbose']) + common.options = common.parse_args(parser) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(MainTest)) diff --git a/tests/metadata.TestCase b/tests/metadata.TestCase index 1a0a1bdc..75dc3b91 100755 --- a/tests/metadata.TestCase +++ b/tests/metadata.TestCase @@ -24,7 +24,7 @@ import fdroidserver from fdroidserver import metadata from fdroidserver.exception import MetaDataException from fdroidserver.common import DEFAULT_LOCALE -from testcommon import TmpCwd, mkdtemp +from testcommon import TmpCwd, mkdtemp, parse_args_for_test def _get_mock_mf(s): @@ -2455,7 +2455,7 @@ if __name__ == "__main__": default=False, help="Spew out even more information than normal", ) - fdroidserver.common.options = parser.parse_args(['--verbose']) + parse_args_for_test(parser, sys.argv) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(MetadataTest)) diff --git a/tests/net.TestCase b/tests/net.TestCase index 59b299b8..a50f5925 100755 --- a/tests/net.TestCase +++ b/tests/net.TestCase @@ -135,7 +135,7 @@ if __name__ == "__main__": default=False, help="Spew out even more information than normal", ) - common.options = parser.parse_args(['--verbose']) + common.options = common.parse_args(parser) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(NetTest)) diff --git a/tests/nightly.TestCase b/tests/nightly.TestCase index 7cb1712d..44e85f04 100755 --- a/tests/nightly.TestCase +++ b/tests/nightly.TestCase @@ -373,7 +373,7 @@ if __name__ == "__main__": default=False, help="Spew out even more information than normal", ) - common.options = parser.parse_args(['--verbose']) + common.options = common.parse_args(parser) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(NightlyTest)) diff --git a/tests/parse-fdroiddata-mirror-config.py b/tests/parse-fdroiddata-mirror-config.py new file mode 100644 index 00000000..f909e910 --- /dev/null +++ b/tests/parse-fdroiddata-mirror-config.py @@ -0,0 +1,13 @@ +#!/usr/bin/env python3 + +import ruamel.yaml + +from pathlib import Path + +mirrors_yml = Path('/home/hans/code/fdroid/fdroiddata/config/mirrors.yml') +with mirrors_yml.open() as fp: + mirrors_config = ruamel.yaml.YAML(typ='safe').load(fp) + +for d in mirrors_config: + d['url'] += '/repo' + print(d, end=',\n') diff --git a/tests/publish.TestCase b/tests/publish.TestCase index 00bbec0e..5ae1fd5f 100755 --- a/tests/publish.TestCase +++ b/tests/publish.TestCase @@ -33,7 +33,7 @@ from fdroidserver import common from fdroidserver import metadata from fdroidserver import signatures from fdroidserver.exception import FDroidException -from testcommon import mkdtemp +from testcommon import mkdtemp, parse_args_for_test class PublishTest(unittest.TestCase): @@ -266,7 +266,8 @@ class PublishTest(unittest.TestCase): os.chdir(self.testdir) - config = common.read_config(Options) + common.options = Options + config = common.read_config() if 'apksigner' not in config: self.skipTest('SKIPPING test_sign_then_implant_signature, apksigner not installed!') config['repo_keyalias'] = 'sova' @@ -340,7 +341,8 @@ class PublishTest(unittest.TestCase): os.chdir(self.testdir) - config = common.read_config(Options) + common.options = Options + config = common.read_config() if 'apksigner' not in config: self.skipTest('SKIPPING test_error_on_failed, apksigner not installed!') config['repo_keyalias'] = 'sova' @@ -422,7 +424,7 @@ if __name__ == "__main__": default=False, help="Spew out even more information than normal", ) - common.options = parser.parse_args(['--verbose']) + parse_args_for_test(parser, sys.argv) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(PublishTest)) diff --git a/tests/rewritemeta.TestCase b/tests/rewritemeta.TestCase index f8388f88..e212ec84 100755 --- a/tests/rewritemeta.TestCase +++ b/tests/rewritemeta.TestCase @@ -273,7 +273,7 @@ if __name__ == "__main__": default=False, help="Spew out even more information than normal", ) - common.options = parser.parse_args(['--verbose']) + common.options = common.parse_args(parser) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(RewriteMetaTest)) diff --git a/tests/scanner.TestCase b/tests/scanner.TestCase index 9069164e..6a85b7f3 100755 --- a/tests/scanner.TestCase +++ b/tests/scanner.TestCase @@ -30,7 +30,7 @@ import fdroidserver.build import fdroidserver.common import fdroidserver.metadata import fdroidserver.scanner -from testcommon import TmpCwd, mkdtemp, mock_open_to_str +from testcommon import TmpCwd, mkdtemp, mock_open_to_str, parse_args_for_test class ScannerTest(unittest.TestCase): @@ -46,8 +46,8 @@ class ScannerTest(unittest.TestCase): self._td.cleanup() def test_scan_source_files(self): - fdroidserver.scanner.options = mock.Mock() - fdroidserver.scanner.options.json = False + fdroidserver.common.options = mock.Mock() + fdroidserver.common.options.json = False source_files = os.path.join(self.basedir, 'source-files') projects = { 'OtakuWorld': 2, @@ -102,8 +102,8 @@ class ScannerTest(unittest.TestCase): """Check for sneaking in banned maven repos""" os.chdir(self.testdir) fdroidserver.scanner.config = None - fdroidserver.scanner.options = mock.Mock() - fdroidserver.scanner.options.json = True + fdroidserver.common.options = mock.Mock() + fdroidserver.common.options.json = True with open('build.gradle', 'w', encoding='utf-8') as fp: fp.write( textwrap.dedent( @@ -135,8 +135,8 @@ class ScannerTest(unittest.TestCase): os.chdir(abs_build_dir) fdroidserver.scanner.config = None - fdroidserver.scanner.options = mock.Mock() - fdroidserver.scanner.options.json = True + fdroidserver.common.options = mock.Mock() + fdroidserver.common.options.json = True keep = [ 'arg.jar', @@ -235,7 +235,7 @@ class ScannerTest(unittest.TestCase): fdroidserver.build.options.scan_binary = False fdroidserver.build.options.notarball = True fdroidserver.build.options.skipscan = False - fdroidserver.scanner.options = fdroidserver.build.options + fdroidserver.common.options = fdroidserver.build.options app = fdroidserver.metadata.App() app.id = 'mocked.app.id' @@ -314,7 +314,7 @@ class ScannerTest(unittest.TestCase): """Check that the scanner can handle scandelete with gradle files with multiple problems""" os.chdir(self.testdir) fdroidserver.scanner.config = None - fdroidserver.scanner.options = mock.Mock() + fdroidserver.common.options = mock.Mock() build = fdroidserver.metadata.Build() build.scandelete = ['build.gradle'] with open('build.gradle', 'w', encoding='utf-8') as fp: @@ -732,15 +732,15 @@ class Test_ScannerTool(unittest.TestCase): refresh.assert_not_called() def test_refresh_true(self): - fdroidserver.scanner.options = mock.Mock() - fdroidserver.scanner.options.refresh_scanner = True + fdroidserver.common.options = mock.Mock() + fdroidserver.common.options.refresh_scanner = True with mock.patch('fdroidserver.scanner.ScannerTool.refresh') as refresh: fdroidserver.scanner.ScannerTool() refresh.assert_called_once() def test_refresh_false(self): - fdroidserver.scanner.options = mock.Mock() - fdroidserver.scanner.options.refresh_scanner = False + fdroidserver.common.options = mock.Mock() + fdroidserver.common.options.refresh_scanner = False with mock.patch('fdroidserver.scanner.ScannerTool.refresh') as refresh: fdroidserver.scanner.ScannerTool() refresh.assert_not_called() @@ -753,8 +753,8 @@ class Test_ScannerTool(unittest.TestCase): refresh.assert_called_once() def test_refresh_options_overrides_config(self): - fdroidserver.scanner.options = mock.Mock() - fdroidserver.scanner.options.refresh_scanner = True + fdroidserver.common.options = mock.Mock() + fdroidserver.common.options.refresh_scanner = True os.chdir(self.testdir) pathlib.Path('config.yml').write_text('refresh_scanner: false') with mock.patch('fdroidserver.scanner.ScannerTool.refresh') as refresh: @@ -824,7 +824,7 @@ if __name__ == "__main__": default=False, help="Spew out even more information than normal", ) - fdroidserver.common.options = parser.parse_args(['--verbose']) + parse_args_for_test(parser, sys.argv) newSuite = unittest.TestSuite() newSuite.addTests( diff --git a/tests/signatures.TestCase b/tests/signatures.TestCase index 5d043a9f..de01c5b9 100755 --- a/tests/signatures.TestCase +++ b/tests/signatures.TestCase @@ -23,7 +23,7 @@ class SignaturesTest(unittest.TestCase): def setUp(self): logging.basicConfig(level=logging.DEBUG) common.config = None - config = common.read_config(common.options) + config = common.read_config() config['jarsigner'] = common.find_sdk_tools_cmd('jarsigner') config['verbose'] = True common.config = config @@ -68,7 +68,7 @@ if __name__ == "__main__": default=False, help="Spew out even more information than normal", ) - common.options = parser.parse_args(['--verbose']) + common.options = common.parse_args(parser) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(SignaturesTest)) diff --git a/tests/signindex.TestCase b/tests/signindex.TestCase index 7931fbdc..f66e9c18 100755 --- a/tests/signindex.TestCase +++ b/tests/signindex.TestCase @@ -37,7 +37,7 @@ class SignindexTest(unittest.TestCase): def setUp(self): signindex.config = None - config = common.read_config(common.options) + config = common.read_config() config['jarsigner'] = common.find_sdk_tools_cmd('jarsigner') config['verbose'] = True config['keystore'] = str(self.basedir / 'keystore.jks') @@ -202,7 +202,7 @@ if __name__ == "__main__": default=False, help="Spew out even more information than normal", ) - common.options = parser.parse_args(['--verbose']) + common.options = common.parse_args(parser) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(SignindexTest)) diff --git a/tests/testcommon.py b/tests/testcommon.py index 069ac42e..2ce9f393 100644 --- a/tests/testcommon.py +++ b/tests/testcommon.py @@ -21,6 +21,7 @@ import tempfile import unittest from pathlib import Path +from unittest import mock class TmpCwd: @@ -72,3 +73,16 @@ def mkdir_testfiles(localmodule, test): testdir = testroot / unittest.TestCase.id(test) testdir.mkdir(exist_ok=True) return tempfile.mkdtemp(dir=testdir) + + +def parse_args_for_test(parser, args): + """Only send --flags to the ArgumentParser, not test classes, etc.""" + + from fdroidserver.common import parse_args + + flags = [] + for arg in args: + if arg[0] == '-': + flags.append(flags) + with mock.patch('sys.argv', flags): + parse_args(parser) diff --git a/tests/update.TestCase b/tests/update.TestCase index bc52981f..dbabcdd0 100755 --- a/tests/update.TestCase +++ b/tests/update.TestCase @@ -60,7 +60,7 @@ import fdroidserver.metadata import fdroidserver.update from fdroidserver.common import CATEGORIES_CONFIG_NAME from fdroidserver.looseversion import LooseVersion -from testcommon import TmpCwd, mkdtemp +from testcommon import TmpCwd, mkdtemp, parse_args_for_test DONATION_FIELDS = ('Donate', 'Liberapay', 'OpenCollective') @@ -1207,7 +1207,7 @@ class UpdateTest(unittest.TestCase): # Set up options fdroidserver.common.options = Options - config = fdroidserver.common.read_config(fdroidserver.common.options) + config = fdroidserver.common.read_config() if 'apksigner' not in config: # TODO remove me for buildserver-bullseye self.skipTest('SKIPPING test_update_with_AllowedAPKSigningKeys, apksigner not installed!') config['repo_keyalias'] = 'sova' @@ -2280,7 +2280,7 @@ if __name__ == "__main__": default=False, help="Spew out even more information than normal", ) - fdroidserver.common.options = parser.parse_args(['--verbose']) + parse_args_for_test(parser, sys.argv) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(UpdateTest)) diff --git a/tests/vcs.TestCase b/tests/vcs.TestCase index 99af14cc..86f67ae1 100755 --- a/tests/vcs.TestCase +++ b/tests/vcs.TestCase @@ -21,7 +21,7 @@ import fdroidserver.build import fdroidserver.common import fdroidserver.metadata import fdroidserver.scanner -from testcommon import mkdtemp +from testcommon import mkdtemp, parse_args_for_test class VCSTest(unittest.TestCase): @@ -97,7 +97,7 @@ if __name__ == "__main__": default=False, help="Spew out even more information than normal", ) - fdroidserver.common.options = parser.parse_args(['--verbose']) + parse_args_for_test(parser, sys.argv) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(VCSTest)) diff --git a/tests/verify.TestCase b/tests/verify.TestCase index cd8fbb6e..eee8e9e8 100755 --- a/tests/verify.TestCase +++ b/tests/verify.TestCase @@ -103,7 +103,7 @@ if __name__ == "__main__": default=False, help="Spew out even more information than normal", ) - common.options = parser.parse_args(['--verbose']) + common.options = common.parse_args(parser) newSuite = unittest.TestSuite() newSuite.addTest(unittest.makeSuite(VerifyTest)) From 5745ed4753ac1ec55006b30dc8052a210f98284e Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Tue, 7 May 2024 17:14:18 +0200 Subject: [PATCH 8/9] common: only try to delete .testfiles dir if it exists Otherwise, some tests fail with an error. --- tests/common.TestCase | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/common.TestCase b/tests/common.TestCase index 5d3b5cdb..48e668fc 100755 --- a/tests/common.TestCase +++ b/tests/common.TestCase @@ -77,7 +77,8 @@ class CommonTest(unittest.TestCase): fdroidserver.common.options = None os.chdir(self.basedir) self._td.cleanup() - shutil.rmtree(self.tmpdir) + if os.path.exists(self.tmpdir): + shutil.rmtree(self.tmpdir) def test_parse_human_readable_size(self): for k, v in ( From 64c9154fffbe9274b22bda4f54c23eee25061ec6 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 8 May 2024 16:36:21 +0200 Subject: [PATCH 9/9] gitlab-ci: fix macOS job after !1466 --- .gitlab-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 8de00bec..b5d9a246 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -362,7 +362,7 @@ macOS: - /bin/bash -n gradlew-fdroid tests/run-tests # TODO remove the packages below once they are included in the Homebrew package - - $(brew --prefix fdroidserver)/libexec/bin/python3 -m pip install biplist pycountry + - $(brew --prefix fdroidserver)/libexec/bin/python3 -m pip install biplist oscrypto pycountry # test fdroidserver from git with current package's dependencies - fdroid="$(brew --prefix fdroidserver)/libexec/bin/python3 $PWD/fdroid" ./tests/run-tests