Revised the code for second pass of PR.

Author: BatuhanSA

README.md

@@ -162,7 +162,9 @@ with each later location overriding any earlier options.
 Here are the places options can be specified in the order that they are checked:
 
 1. Global Configuration --
-`<platform-specific user config location>/autograder.json` which is considered the "proper" place to store user-related config for the platform you are using (according to [platformdirs](https://github.com/platformdirs/platformdirs)). Use `--help` to see the exact place in your specific case. This is a great place to store login credentials.
+`<platform-specific user config location>/autograder.json` which is considered the "proper" place to store user-related config
+for the platform you are using (according to [platformdirs](https://github.com/platformdirs/platformdirs)).
+Use `--help` to see the exact place in your specific case. This is a great place to store login credentials.
 
 2. Local Configuration --
    Local configuration files can be found in different locations, the first file found will be used and no other locations are searched.
@@ -179,7 +181,7 @@ Here are the places options can be specified in the order that they are checked:
 
 A base config file (`autograder.json`) is often distributed with assignments that contains most the settings you need.
 You can modify this config to include your settings and use that for setting all your configuration options.
-A `autograder.json` file may look something like:
+An `autograder.json` file may look something like:
 ```json
 {
     "course": "my-course",

autograder/api/config.py

@@ -15,6 +15,9 @@
 DEFAULT_GLOBAL_CONFIG_PATH = platformdirs.user_config_dir('autograder.json')
 CSV_TO_LIST_DELIMITER = ','
 MAP_KEY_VALUE_SEP = '='
+DEPTH_LIMIT = 10000
+CONFIG_TYPE_DELIMITER = "::"
+
 
 class APIParam(object):
     def __init__(self, key, description,
@@ -99,36 +102,37 @@ def _parse_api_config(config, params, additional_required_keys, additional_optio
 
     return data, extra
 
-# The local_config_search_limit input limits config search depth.
-# This helps to prevent detection of autograder.json files in higher directories during testing.
-def get_local_config_path(local_config_search_limit = None):
+def get_local_config_path(local_config_root_cutoff = None):
     """
-    Searches for a configuration file in a hierarchical order, starting with ./autograder.json,
-    then ./config.json, and continuing up the directory tree looking for autograder.json.
+    Searches for a configuration file in a hierarchical order,
+    starting with DEFAULT_CONFIG_FILENAME, then LEGACY_CONFIG_FILENAME,
+    and continuing up the directory tree looking for DEFAULT_CONFIG_FILENAME.
     Returns the path to the first valid configuration file found.
-    If no configuration file is found, returns None.
+
+    If no configuration file is found, returns None. The cutoff limits config search depth.
+    This helps to prevent detection of DEFAULT_CONFIG_FILENAME files in higher directories during testing.
     """
 
-    # autograder.json file in current directory
+    # The case where DEFAULT_CONFIG_FILENAME file in current directory.
     if (os.path.isfile(DEFAULT_CONFIG_FILENAME)):
         return os.path.abspath(DEFAULT_CONFIG_FILENAME)
 
-    # config.json file in current directory
+    # The case where LEGACY_CONFIG_FILENAME file in current directory.
     if (os.path.isfile(LEGACY_CONFIG_FILENAME)):
         return os.path.abspath(LEGACY_CONFIG_FILENAME)
 
-    #  An autograder.json file located in any ancestor directory on the path to root.
+    #  The case where a DEFAULT_CONFIG_FILENAME file located in any ancestor directory on the path to root.
     parent_dir = os.path.dirname(os.getcwd())
     return _get_ancestor_config_file_path(
         parent_dir,
-        local_config_search_limit = local_config_search_limit
+        local_config_root_cutoff = local_config_root_cutoff
     )
 
 def get_tiered_config(
         cli_arguments,
         skip_keys = [CONFIG_PATHS_KEY],
         global_config_path = DEFAULT_GLOBAL_CONFIG_PATH,
-        local_config_search_limit = None):
+        local_config_root_cutoff = None):
     """
     Get all the tiered configuration options (from files and CLI).
     If |show_sources| is True, then an addition dict will be returned that shows each key,
@@ -146,8 +150,8 @@ def get_tiered_config(
         _load_configs_and_sources(global_config_path, config, sources, "<global config file>")
 
     # Check the local user config file.
-    local_config_path = get_local_config_path(local_config_search_limit = local_config_search_limit)
-    if local_config_path is not None:
+    local_config_path = get_local_config_path(local_config_root_cutoff = local_config_root_cutoff)
+    if (local_config_path is not None):
         _load_configs_and_sources(local_config_path, config, sources, "<local config file>")
 
     # Check the config file specified on the command-line.
@@ -220,38 +224,39 @@ def _load_configs_and_sources(config_path, config, sources, type_of_config):
     with open(config_path, 'r') as file:
         for key, value in json.load(file).items():
             config[key] = value
-            sources[key] = "%s::%s" % (type_of_config, os.path.abspath(config_path))
+            sources[key] = f"{type_of_config}::{os.path.abspath(config_path)}"
 
 def _get_ancestor_config_file_path(
         current_directory,
         config_file = DEFAULT_CONFIG_FILENAME,
-        local_config_search_limit = None):
+        local_config_root_cutoff = None):
     """
-    Search through the parent directories (until root) for a configuration file.
-    Stops at the first occurrence of the specified config file
+    Search through the parent directories (until root or a given cutoff directory)
+    for a configuration file. Stops at the first occurrence of the specified config file
     (default: DEFAULT_CONFIG_FILENAME) along the path to root.
     Returns the path if a configuration file is found.
     Otherwise, returns None.
     """
 
-    while True:
-        parent_dir = os.path.dirname(current_directory)
-        config_file_path = os.path.join(current_directory, config_file)
+    current_directory = os.path.abspath(current_directory)
 
+    # DEPTH_LIMIT ensures termination on filesystems where the root directory is not equal to its parent.
+    for _ in range(DEPTH_LIMIT):
+        config_file_path = os.path.join(current_directory, config_file)
         if (os.path.isfile(config_file_path)):
             return config_file_path
 
-        if local_config_search_limit == current_directory:
+        if (local_config_root_cutoff == current_directory):
             break
 
-        if parent_dir == current_directory:
+        parent_dir = os.path.dirname(current_directory)
+        if (parent_dir == current_directory):
             break
 
-        current_directory = os.path.dirname(current_directory)
+        current_directory = parent_dir
 
     return None
 
-
 def _submission_add_func(parser, param):
     parser.add_argument('submissions', metavar = 'SUBMISSION',
         action = 'store', type = str, nargs = '+',

autograder/cli/config/list.py

@@ -5,22 +5,21 @@
 DESCRIPTION = "List your current configuration options."
 
 def run(args):
-    arguments = {
-        "cli_arguments": args,
-        "skip_keys": [
+    config, sources = autograder.api.config.get_tiered_config(
+        cli_arguments = args,
+        skip_keys = [
             'show_origin', 'verbose',
             autograder.api.config.CONFIG_PATHS_KEY, 'global_config_path',
         ],
-        "global_config_path": args.global_config_path
-    }
+        global_config_path = args.global_config_path
+    )
 
     config_list = []
-    config, sources = autograder.api.config.get_tiered_config(**arguments)
-    for key, value in config.items():
+    for (key, value) in config.items():
         config_str = ''
         if args.show_origin:
             raw_source = sources.get(key)
-            source_path = raw_source.split("::")[1]
+            source_path = raw_source.split(autograder.api.config.CONFIG_TYPE_DELIMITER)[1]
             config_str = source_path + "\t"
 
         config_str += "%s: %s" % (key, value)
@@ -35,11 +34,11 @@ def _get_parser():
         skip_server = True)
 
     parser.add_argument("--show-origin", dest = 'show_origin',
-        action = 'store_true', help = 'Show origin of configs.')
+        action = 'store_true', help = "Shows where each configuration's value was obtained from.")
 
     parser.add_argument("--global-config", dest = 'global_config_path',
         action = 'store', type = str, default = autograder.api.config.DEFAULT_GLOBAL_CONFIG_PATH,
-        help = 'Path to the global configuration file. (default: %(default)s)')
+        help = 'Path to the global configuration file (default: %(default)s).')
 
     return parser
 

autograder/util/dirent.py

@@ -18,7 +18,7 @@ def get_temp_path(prefix = '', suffix = '', rm = True):
     if (rm):
         atexit.register(remove, path)
 
-    return os.path.realpath(path)
+    return path
 
 def remove(path):
     if (not os.path.exists(path)):

tests/base.py

@@ -8,6 +8,8 @@ class BaseTest(unittest.TestCase):
     A base class for tests.
     """
 
+    maxDiff = None
+
     def assertListEqual(self, a, b):
         a_json = json.dumps(a, indent = 4)
         b_json = json.dumps(b, indent = 4)

tests/cli/test_cli.py

@@ -18,6 +18,7 @@
 
 TEST_CASE_SEP = '---'
 TEMP_DIR_ID = '__TEMP_DIR__'
+ABS_DATA_DIR_ID = '__ABS_DATA_DIR__'
 
 DEFAULT_OUTPUT_CHECK = 'content_equals'
 
@@ -107,8 +108,9 @@ def _load_allowed_arg_keys(self, module_name):
 
 def _prepare_string(text, temp_dir):
     replacements = [
-        (tests.server.base.DATA_DIR_ID, os.path.abspath(DATA_DIR)),
+        (tests.server.base.DATA_DIR_ID, DATA_DIR),
         (TEMP_DIR_ID, temp_dir),
+        (ABS_DATA_DIR_ID, os.path.abspath(DATA_DIR))
     ]
 
     for (key, base_dir) in replacements:

tests/cli/testdata/config/config_list_sources.txt

@@ -7,4 +7,4 @@
     ]
 }
 ---
-__DATA_DIR__(configs/simple/autograder.json)	user: user@test.edulinq.org
+__ABS_DATA_DIR__(configs/simple/autograder.json)	user: user@test.edulinq.org

tests/server/base.py

@@ -15,8 +15,6 @@ class ServerBaseTest(tests.base.BaseTest):
     A base tests that need to call the mock server.
     """
 
-    maxDiff = None
-
     _server_process = None
     _port = None