Skip to content

Review: Return false when dotenv contains only valueless declarations - #2

Draft
srijan2607 wants to merge 1 commit into
mainfrom
fix/valueless-load-result-reissued
Draft

srijan2607 wants to merge 1 commit into
mainfrom
fix/valueless-load-result-reissued

Conversation

@srijan2607

Copy link
Copy Markdown
Owner

Problem and bounded change

load_dotenv(stream=StringIO("EXAMPLE_KEY")) returns True even though a bare declaration has no value and is ignored. Return False when every parsed value is None, as for an empty configuration. Empty-string assignments remain valid.

Only src/dotenv/main.py and tests/test_load_result.py change. Review the predicate against the existing if v is not None assignment guard, and inspect the file/stream, override, ambient-environment and empty-string cases. Parsing, interpolation, environment contents and discovery are unchanged. Valued declarations skipped by override=False still return True; this is not a general redesign of the return contract.

This is a public draft targeting my own fork's main. It replaces the earlier fork review PR with the same corrected code commit. No source changes were made during reissue. Keep it as a draft pending CI verification; local passing tests are not a CI pass. Personal human review is not claimed.

  • Base: 0b2880591780a426b0551436f47a17e7a76d954f
  • Exact head: 0f7609f116b19ac7355daa6b5d9147a8c5817907
  • Review the proposed official title/body in the separate preview comment.

Portable clean-checkout review

Requires Git, uv and Python 3.13. Use a fresh directory without an unrelated ancestor .env; discovery tests may otherwise see it. The following setup is a portable recipe, not a claim that a fresh installation was rerun during this reissue. Current execution reused the existing isolated environment and installed console script.

git clone --branch fix/valueless-load-result-reissued https://github.com/srijan2607/python-dotenv.git
cd python-dotenv
git checkout 0f7609f116b19ac7355daa6b5d9147a8c5817907
uv venv --python 3.13 .venv
uv pip install --python .venv/bin/python 'pytest==9.1.1' 'click==8.5.0' 'ruff==0.12.0'
uv pip install --python .venv/bin/python --no-deps -e .
export PATH="$PWD/.venv/bin:$PATH"
export PYTHONPATH="$PWD/src"
.venv/bin/python -m pytest tests/test_load_result.py -q
.venv/bin/python -m pytest tests -q -rs --basetemp .test-tmp
.venv/bin/ruff check .
.venv/bin/ruff format --check .
git diff --check HEAD^

Expected focused result: 22 passed. Current full-suite result: 306 passed, 1 skipped (optional IPython missing), with 32 existing Click deprecation warnings.

Minimal reproduction:

.venv/bin/python -c 'from io import StringIO; from dotenv import load_dotenv; print(load_dotenv(stream=StringIO("EXAMPLE_KEY"))); print(load_dotenv(stream=StringIO("EXAMPLE_KEY=")))'

Before: True, True. After: False, True.

Exact-base negative control

After setup, copy the same regression tests into a pristine base worktree:

git worktree add --detach ../python-dotenv-before 0b2880591780a426b0551436f47a17e7a76d954f
cp tests/test_load_result.py ../python-dotenv-before/tests/test_load_result.py
PYTHONPATH="$PWD/../python-dotenv-before/src" .venv/bin/python -m pytest ../python-dotenv-before/tests/test_load_result.py -q --basetemp .before-test-tmp

Expected nonzero exit: 16 failed, 6 passed. The bare-only false-return assertions fail against the original production code; the positive controls pass.

Verification and limits

Rerun on this exact head with CPython 3.13.13, pytest 9.1.1, Click 8.5.0 and Ruff 0.12.0: 22 focused tests passed; full suite 306 passed/1 skipped; exact-base negative control 16 failed/6 passed; lint, format and whitespace checks passed. The full-suite run used the actual installed dotenv console script. The independently reviewed source and regression blob IDs match this head exactly, with no blocking code finding in that review. No personal human review or maintainer approval is asserted.

Optional IPython, Windows and other Python versions were not tested locally. CI must be evaluated separately at the current head. The contribution instructions welcome an issue or PR; no issue-first requirement, mandatory disclosure, CLA/DCO or PR template was found in the inspected current policy files.

Implemented and tested with AI assistance; independently reviewed by a separate AI agent. Pending Srijan's human review; not submitted upstream.
@srijan2607

srijan2607 commented Oct 15, 2026 •

Copy link
Copy Markdown
Owner Author

Official submission preview

Exact title

Return false when dotenv contains only valueless declarations

Exact body

load_dotenv() returns True for content containing only bare declarations such as EXAMPLE_KEY, although those declarations are ignored and cannot set environment variables.

Return False when all parsed values are None, matching the existing empty-configuration result. Empty-string assignments remain valid, and valued assignments skipped by override=False retain their existing result. Parsing, interpolation and environment contents are unchanged.

Add 22 regression cases covering file/stream inputs, both override modes, existing environment values, bare-only declarations, empty assignments and mixed content.

Testing on Python 3.13.13:

  • Original implementation with the new tests: 16 failed, 6 passed (expected negative control).
  • Patched regression tests: 22 passed.
  • Full suite: 306 passed, 1 skipped (optional IPython unavailable).
  • Ruff lint/format checks and git diff --check passed.

Kept as a draft pending CI verification; local results are not a CI pass. Windows and other Python versions have not been tested locally.

Prerequisite issue

None: current CONTRIBUTING welcomes an issue or a pull request. No issue-first requirement was found.

Submitted as theskumar#733 (draft). CI is not represented as green; no review request has been sent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant