Improve addopts masking (fixes repos with addopts which interfere with cf)

[aseembits93]

User description

Fixes issues with dd-trace-py, still need to test, rough on the edges

---

PR Type

Enhancement, Tests

---

Description

• Support multiple pytest config files

• Filter blacklisted addopts arguments

• Safely modify and restore configs

• Add comprehensive tests

---

Diagram Walkthrough

flowchart LR
  A["get_all_closest_config_files"] -- "returns Paths" --> B["custom_addopts context"]
  B -- "for each file" --> C["modify_addopts"]
  C -- "parse TOML/INI" --> D["filter_args"]
  D -- "remove blacklisted flags" --> E["write updated addopts"]
  B -- "on exit" --> F["restore original content if modified"]

Loading

File Walkthrough

	Relevant files
Enhancement	code_utils.py Robust multi-file addopts filtering and restoration            codeflash/code_utils/code_utils.py Add blacklist and args filter for addopts. Implement modify_addopts for TOML/INI files. Extend custom_addopts to process multiple configs. Improve error tolerance and restoration logic. +95/-29  config_parser.py Discover and cache closest pytest config files                      codeflash/code_utils/config_parser.py Cache closest config files per cwd. Add find_closest_config_file helper. Add get_all_closest_config_files aggregator. +29/-0
Tests	test_code_utils.py Tests for addopts filtering and safe restoration                  tests/code_utils/test_code_utils.py Add tests for TOML/INI modify/restore. Cover missing sections and no files cases. Validate multi-file handling and exception safety. +190/-0

---

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit b7dc7bd)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Possible Issue The condition current_arg.startswith(BLACKLIST_ADDOPTS) treats BLACKLIST_ADDOPTS as a tuple for startswith, which can unintentionally match flags that only share a prefix (e.g., -nX). Also, for long options with inline values (e.g., --cov=src), the code removes only the flag token when split produced a single token, but the value is inline and should be removed too; verify that all blacklisted patterns including inline assignments are fully filtered. if current_arg.startswith(BLACKLIST_ADDOPTS): # Skip this argument i += 1 # Check if the next argument is a value (doesn't start with -) if i < len(addopts_args) and not addopts_args[i].startswith("-"): # Skip the value as well i += 1 else: # Keep this argument filtered_args.append(current_arg) i += 1 return filtered_args INI Parsing Risk Using configparser.read_string on arbitrary INI content may fail on files with encoding or interpolation; consider disabling interpolation (ConfigParser(interpolation=None)) to avoid accidental variable expansion and ensure round-trip fidelity when writing back. config = configparser.ConfigParser() config.read_string(content) data = {section: dict(config[section]) for section in config.sections()} if config_file.name in {"pytest.ini", ".pytest.ini", "tox.ini"}: original_addopts = data.get("pytest", {}).get("addopts", "") # should only be a string else: original_addopts = data.get("tool:pytest", {}).get("addopts", "") # should only be a string original_addopts = original_addopts.replace("=", " ") addopts_args = original_addopts.split() new_addopts_args = filter_args(addopts_args) if new_addopts_args == addopts_args: return content, False # change addopts now if file_type == ".toml": data["tool"]["pytest"]["ini_options"]["addopts"] = " ".join(new_addopts_args) # Write modified file with Path.open(config_file, "w", encoding="utf-8") as f: f.write(tomlkit.dumps(data)) return content, True elif config_file.name in {"pytest.ini", ".pytest.ini", "tox.ini"}: config.set("pytest", "addopts", " ".join(new_addopts_args)) # Write modified file with Path.open(config_file, "w", encoding="utf-8") as f: config.write(f) return content, True else: config.set("tool:pytest", "addopts", " ".join(new_addopts_args)) # Write modified file with Path.open(config_file, "w", encoding="utf-8") as f: config.write(f) Cache Keying The ALL_CONFIG_FILES cache keys on the initial cwd (cur_path) but not on file_type alone; changes to cwd within the same process or tests could cause stale/misleading results. Validate cache invalidation or tie cache strictly to absolute search root. if cur_path in ALL_CONFIG_FILES and file_type in ALL_CONFIG_FILES[cur_path]: return ALL_CONFIG_FILES[cur_path][file_type] while dir_path != dir_path.parent: config_file = dir_path / file_type if config_file.exists(): if cur_path not in ALL_CONFIG_FILES: ALL_CONFIG_FILES[cur_path] = {} ALL_CONFIG_FILES[cur_path][file_type] = config_file return config_file # Search for pyproject.toml in the parent directories dir_path = dir_path.parent return None

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to b7dc7bd
Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Make filtering precise and robust Update the filtering to use exact-match for short flags like -n and keep the current prefix behavior for long options. Also correctly skip values passed via --opt=value without converting = to spaces. This prevents accidental removals and preserves original tokenization. codeflash/code_utils/code_utils.py [93-104] def filter_args(addopts_args: list[str]) -> list[str]: - filtered_args = [] + filtered_args: list[str] = [] i = 0 while i < len(addopts_args): - current_arg = addopts_args[i] - # Check if current argument starts with --cov - if current_arg.startswith(BLACKLIST_ADDOPTS): - # Skip this argument + arg = addopts_args[i] + + # Exact-match short flags + if arg in BLACKLIST_EXACT: i += 1 - # Check if the next argument is a value (doesn't start with -) + # If next token is a value (doesn't start with '-') treat it as the flag's value and skip it if i < len(addopts_args) and not addopts_args[i].startswith("-"): - # Skip the value as well i += 1 - else: - # Keep this argument - filtered_args.append(current_arg) + continue + + # Long options possibly with attached value via '=' + if any(arg == opt or arg.startswith(opt + "=") for opt in BLACKLIST_ADDOPTS): i += 1 + # If value is in the next token (space-separated), skip it too + if not ("=" in arg) and i < len(addopts_args) and not addopts_args[i].startswith("-"): + i += 1 + continue + + filtered_args.append(arg) + i += 1 return filtered_args Suggestion importance[1-10]: 7 __ Why: Improves correctness by handling exact-match short flags and '--opt=value' without altering tokens, aligning with tests and avoiding unintended removals; solid enhancement though not a critical bug fix.	Medium
	Preserve equals in arguments Replacing '=' with spaces changes semantics and breaks --key=value parsing; it can also corrupt values. Tokenize without altering '=' and handle --opt=value during filtering. This preserves original content and avoids malformed rewrites. codeflash/code_utils/code_utils.py [126-130] if filename == "pyproject.toml": - # use tomlkit data = tomlkit.parse(content) original_addopts = data.get("tool", {}).get("pytest", {}).get("ini_options", {}).get("addopts", "") - # nothing to do if no addopts present if original_addopts == "": return content, False if isinstance(original_addopts, list): - original_addopts = " ".join(original_addopts) - original_addopts = original_addopts.replace("=", " ") - addopts_args = ( - original_addopts.split() - ) # any number of space characters as delimiter, doesn't look at = which is fine + original_addopts = " ".join(x.strip() for x in original_addopts if x is not None) + # Do not replace '='; split on whitespace to preserve --opt=value tokens + addopts_args = original_addopts.split() Suggestion importance[1-10]: 7 __ Why: Accurately identifies that replacing '=' can corrupt arguments and suggests preserving it, which prevents malformed rewrites and complements improved filtering; meaningful but not critical.	Medium
	Prevent overbroad flag matching The blacklist tuple is used with str.startswith, but mixing long options with a short flag like -n will also remove any other short flags starting with -n (e.g., -no, -nox). Constrain matching for -n to the exact flag only. This avoids inadvertently stripping unrelated arguments. codeflash/code_utils/code_utils.py [23] -BLACKLIST_ADDOPTS = ("--benchmark", "--sugar", "--codespeed", "--cov", "--profile", "--junitxml", "-n") +BLACKLIST_ADDOPTS = ("--benchmark", "--sugar", "--codespeed", "--cov", "--profile", "--junitxml") +BLACKLIST_EXACT = {"-n"} Suggestion importance[1-10]: 6 __ Why: Correctly notes that using startswith with a tuple including '-n' may overmatch and proposes separating exact-match short flags, improving precision; impact is moderate and requires coordinating changes in filtering logic.	Low

---

Previous suggestions

Suggestions up to commit 333271c

Category	Suggestion	Impact
Possible issue	Fix incorrect suffix check The suffix check misses ".cfg" because "cfg" lacks the leading dot, causing valid setup.cfg files to be skipped. Fix the set to include ".cfg" and ensure consistency with lower-cased suffixes. codeflash/code_utils/code_utils.py [110-111] -if file_type not in {".toml", ".ini", "cfg"} or not config_file.exists(): +if file_type not in {".toml", ".ini", ".cfg"} or not config_file.exists(): return "", False Suggestion importance[1-10]: 8 __ Why: The current set includes "cfg" without a dot, so .cfg files are skipped; adding the missing dot is a concrete correctness fix that prevents silently ignoring valid setup.cfg files.	Medium
	Prevent overbroad flag matches Using startswith with this tuple will also match arguments like "--cov-report" or "--profile-svg" unintentionally. Narrow the blacklist to exact flags that either have separate values or are the top-level flags you intend to strip. For flags that accept a value via "=" keep them, but avoid catching unrelated prefixed options. codeflash/code_utils/code_utils.py [23] -BLACKLIST_ADDOPTS = ("--benchmark", "--sugar", "--codespeed", "--cov", "--profile", "--junitxml", "-n") +BLACKLIST_ADDOPTS = { + "--benchmark", + "--sugar", + "--codespeed", + "--cov", + "--profile", + "--junitxml", + "-n", +} Suggestion importance[1-10]: 7 __ Why: filter_args uses startswith(BLACKLIST_ADDOPTS), so a tuple causes unintended matches like --cov-report; narrowing to exact flags (e.g., using a set and equality checks) avoids false positives and improves correctness.	Medium
	Correct INI section handling Writing to "tool:pytest" in INI files is nonstandard and may create a new section instead of updating existing "pytest" options in "setup.cfg". Detect "setup.cfg" explicitly and write under the "tool:pytest" section only for that file; otherwise use "pytest". Also guard for missing sections before assignment. codeflash/code_utils/code_utils.py [128-159] if filename == "pyproject.toml": ... else: # use configparser config = configparser.ConfigParser() config.read_string(content) - data = {section: dict(config[section]) for section in config.sections()} - if config_file.name in {"pytest.ini", ".pytest.ini", "tox.ini"}: - original_addopts = data.get("pytest", {}).get("addopts", "") # should only be a string - else: - original_addopts = data.get("tool:pytest", {}).get("addopts", "") # should only be a string + target_section = "pytest" if config_file.name in {"pytest.ini", ".pytest.ini", "tox.ini"} else "tool:pytest" + if not config.has_section(target_section): + config.add_section(target_section) + original_addopts = config.get(target_section, "addopts", fallback="") addopts_args = original_addopts.split() + new_addopts_args = filter_args(addopts_args) -... -elif config_file.name in {"pytest.ini", ".pytest.ini", "tox.ini"}: - config["pytest"]["addopts"] = " ".join(new_addopts_args) - # Write modified file +if new_addopts_args == addopts_args: + return content, False + +if filename == "pyproject.toml": + data["tool"]["pytest"]["ini_options"]["addopts"] = " ".join(new_addopts_args) + with Path.open(config_file, "w", encoding="utf-8") as f: + f.write(tomlkit.dumps(data)) + return content, True +else: + config.set(target_section, "addopts", " ".join(new_addopts_args)) with Path.open(config_file, "w", encoding="utf-8") as f: config.write(f) - return content, True -else: - config["tool:pytest"]["addopts"] = " ".join(new_addopts_args) - # Write modified file - with Path.open(config_file, "w", encoding="utf-8") as f: - config.write(f) - return content, True + return content, True Suggestion importance[1-10]: 7 __ Why: The suggestion correctly unifies reading/writing INI via a target section and ensures sections exist, aligning with standard handling for pytest.ini vs setup.cfg; this improves robustness though the overall logic remains similar.	Medium

[github-actions]

Persistent review updated to latest commit b7dc7bd

[codeflash-ai]

This PR is now faster! 🚀 Aseem Saxena accepted my code suggestion above.