Skip to content

FIX: re-raise what concurrency test workers throw #786

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
d-chambers merged 3 commits into from
Jul 27, 2026
Merged

FIX: re-raise what concurrency test workers throw #786

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1b45d94
Re-raise what concurrency test workers throw
d-chambers Jul 26, 2026
875f15d
Re-raise the first worker failure, not an ExceptionGroup
d-chambers Jul 26, 2026
7304893
Drop the tests for the test helper
d-chambers Jul 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions tests/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,23 +117,33 @@ def run_in_threads():

A barrier releases every thread together, so concurrency tests do not
need sleeps. The timeouts turn a deadlock into a failure rather than a
hung test run.
hung test run, and anything a worker raises is re-raised here rather
than being printed while the test carries on with a None result.
"""

def _run(func, count=4, timeout=60):
barrier = threading.Barrier(count, timeout=timeout)
results = [None] * count
errors = []

def worker(index):
barrier.wait()
results[index] = func(index)
try:
barrier.wait()
results[index] = func(index)
except BaseException as error: # re-raised in the calling thread
errors.append(error)

threads = [threading.Thread(target=worker, args=(i,)) for i in range(count)]
for thread in threads:
thread.start()
for thread in threads:
thread.join(timeout)
assert not thread.is_alive(), "thread never finished; possible deadlock"

@coderabbitai coderabbitai Bot Jul 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== repo files matching conftest.py =="
fd -a 'conftest\.py$' . | sed 's#^\./##' || true

echo "== tests/conftest.py around reported line =="
if [ -f tests/conftest.py ]; then
  wc -l tests/conftest.py
  sed -n '110,170p' tests/conftest.py | nl -ba -v110
else
  echo "tests/conftest.py not found"
fi

echo "== assert usage in tests/conftest.py =="
rg -n '\bassert |\bassertion|is_alive|threading|Thread|daemon' tests/conftest.py || true

echo "== Python assert optimization behavior probe =="
python3 - <<'PY'
import tempfile, os, subprocess

for opt in ("", "-O"):
    ok = False
    for stmt in ("if thread.is_alive(): raise AssertionError('alive')", "assert not thread.is_alive(), 'alive'"):
        with tempfile.NamedTemporaryFile("w", suffix=".py", delete=False) as f:
            f.write(stmt + "\n")
            f.flush()
            p = subprocess.run([opt, "python3", f.name], capture_output=True, text=True)
            print({
                "opt_flag": opt,
                "stmt": stmt,
                "rc": p.returncode,
                "stdout": p.stdout.strip(),
                "stderr": p.stderr.strip(),
            })
        os.unlink(f.name)
PY

Repository: DASDAE/dascore

Length of output: 426

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== tests/conftest.py around reported line =="
if [ -f tests/conftest.py ]; then
  awk 'NR>=110 && NR<=170 { printf "%7d\t%s\n", NR, $0 }' tests/conftest.py
else
  echo "tests/conftest.py not found"
fi

echo "== assert usage in tests/conftest.py =="
rg -n '\bassert |\bassertion|is_alive|threading|Thread|daemon' tests/conftest.py || true

echo "== Python assert optimization behavior probe =="
python3 - <<'PY'
import tempfile, os, subprocess

for opt in ("", "-O"):
    for stmt in ("if thread.is_alive(): raise AssertionError('alive')", "assert not thread.is_alive(), 'alive'"):
        with tempfile.NamedTemporaryFile("w", suffix=".py", delete=False) as f:
            f.write(stmt + "\n")
            f.flush()
            p = subprocess.run([opt, "python3", f.name], capture_output=True, text=True)
            print({
                "opt_flag": opt,
                "stmt": stmt,
                "rc": p.returncode,
                "stdout": p.stdout.strip(),
                "stderr": p.stderr.strip(),
            })
        os.unlink(f.name)
PY

Repository: DASDAE/dascore

Length of output: 4055

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
import tempfile, os, subprocess, threading, time

def test_assert_case(name, stmt):
    path = tempfile.mktemp()
    code = f"thread = threading.Thread(target=time.sleep, args=(0.01,))\nthread.start()\nthread.join(0)\n{stmt}\nprint('ran')\n"
    with open(path, "w") as f:
        f.write(code)
    try:
        for opt in ("", "-O"):
            cmd = [opt, "python3", path]
            p = subprocess.run(cmd, capture_output=True, timeout=5)
            print(f"{name}: opt={op r or '<empty>'} rc={p.returncode} stdout={p.stdout!r} stderr={p.stderr!r}")
        print(f"{name}: assertion removed with -O = 'ran' not found")
    finally:
        os.unlink(path)

test_assert_case("alive-check", "if thread.is_alive(): raise AssertionError('alive')")
test_assert_case("assert-check", "assert not thread.is_alive(), 'alive'")
PY

Repository: DASDAE/dascore

Length of output: 292

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
import tempfile, os, subprocess, threading, time

def run_case(name, stmt):
    path = tempfile.mktemp()
    code = f"thread = threading.Thread(target=time.sleep, args=(0.01,))\nthread.start()\nthread.join(0)\n{stmt}\nprint('ran')\n"
    with open(path, "w") as f:
        f.write(code)
    try:
        for opt in ("", "-O"):
            cmd = [opt + "python3", path] if opt else ["python3", path]
            p = subprocess.run(cmd, capture_output=True, timeout=5)
            opt_repr = "<empty>" if not opt else opt
            print(f"{name}: opt={opt_repr} rc={p.returncode} stdout={p.stdout!r} stderr={p.stderr!r}")
    finally:
        os.unlink(path)

run_case("alive-check", "if thread.is_alive(): raise AssertionError('alive')")
run_case("assert-check", "assert not thread.is_alive(), 'alive'")
PY

Repository: DASDAE/dascore

Length of output: 1020

🌐 Web query:

Python optimize -O removes assert statements documentation

💡 Result:

In Python, assert statements are designed to be used for debugging and testing purposes rather than for mandatory runtime logic [1][2]. When Python is run in optimized mode, the interpreter effectively removes these statements [1]. How to remove assert statements: You can disable assert statements by running the Python interpreter in optimized mode using one of the following methods [1]: 1. Command-line options: - Use the -O flag: This sets the built-in debug constant to False and removes assert statements from the compiled bytecode [1]. - Use the -OO flag: This performs the same optimization as -O, but also discards docstrings from the compiled bytecode [1]. 2. Environment variable: - Set the PYTHONOPTIMIZE environment variable to a non-empty string (e.g., 1 or 2) to achieve the same effect as the -O or -OO flags, respectively [1]. Important Considerations: - Assertions are not safe for production runtime checks because they are silently skipped when optimized mode is enabled [2]. Code required for program logic or security (such as input validation or permission checks) should never be placed within an assert statement [2]. - If you need to ensure code always runs, regardless of optimization settings, use standard conditional if statements instead [2]. - The behavior is linked to the debug constant; when this is False, any code explicitly written within an if debug: block will also be skipped, in addition to assert statements [1].

Citations:

Do not rely on assert for deadlock detection.

With Python optimization turned on, line 141 is removed, so live workers can pass this check and return partial results or keep non-daemon threads alive. Use if thread.is_alive(): raise AssertionError("thread never finished; possible deadlock") instead.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/conftest.py` at line 141, Replace the assert-based completion check in
the thread cleanup logic with an explicit is_alive() conditional that raises
AssertionError("thread never finished; possible deadlock") when the worker
remains alive, ensuring the check remains active under Python optimization.

if errors:
# The first failure is re-raised as-is rather than aggregated:
# ExceptionGroup is 3.11+ while the package supports 3.10, and it
# rejects BaseException members such as pytest's own skip/fail.
raise errors[0]
return results

return _run
Expand Down
Loading