Extract nothing until side effects have a frozen contract. A messy path helper often creates directories while building strings. Split that helper too early and a swallowed OSError disappears.
This walkthrough pins current behavior before any edit. Then it applies one small reversible change only. The samples are labeled proposals, not executed runs.
Why this extract fails in review
Legacy report code often mixes three jobs in one function. It joins path parts into one returned string. It creates a parent directory as a side effect.
It also hides permission errors from every caller. A rename looks harmless in a small diff. A later import can change when that directory appears.
Characterization tests catch that drift before the extract. They record what the code does on today's inputs. They do not record what a cleaner design would do.
The messy helper
The function below is an unexecuted teaching sample. Confirm it against your tree before you trust it. Do not paste it over a live module.
# proposal: unexecuted sample, confirm on your tree
import os
from pathlib import Path
def build_report_path(root, name, stamp, make=True):
raw = os.path.join(str(root or ''), str(name or 'report'))
if stamp:
raw = raw + '-' + str(stamp)
path = Path(raw)
if make:
try:
path.parent.mkdir(parents=True, exist_ok=True)
except OSError:
return None
return str(path)
This function returns either a string or None. None means mkdir failed, not a missing name. Callers may branch on that exact None result.
Blank root becomes an empty string before the join. A missing name becomes the literal word report. A missing stamp skips the suffix entirely here.
Integer zero is also a missing stamp here. In Python, zero is falsy, so the branch skips. The text zero is truthy and still appends.
That distinction is a contract, not a style nit. A cleanup that treats zero as present will move files. Pin that distinction before you extract the join.
What to freeze
Do not freeze comments, quotes, or line length. Freeze observable results that callers can already see. Four caller-visible outcomes matter for this path helper.
- A normal call returns one stable path string.
- Empty root and blank name still follow today's join rules.
- A failed mkdir returns None and does not raise.
- A false make flag skips creation and still returns a path.
A later extract may keep the same strings. It can still move the mkdir call earlier. That move is a behavior change even when strings match.
An existing parent must remain a successful mkdir. The exist_ok flag already treats that directory as fine. Do not convert that success into a new error.
Decision table
Use this table before you edit a line. Each row names one observation from the current helper. The last column limits what the first change may do.
| Observation | Current result | First change allowed |
|---|---|---|
| root set, name q, stamp 01 | string ending in q-01 | keep that exact string |
| root empty, name empty, stamp missing | string ending in report | keep today's join rules |
| parent path is a file, make true | None, no exception | keep swallowed OSError |
| make false | string, no new directory | do not add a mkdir |
| stamp integer 0 | no suffix, because 0 is falsy | do not treat 0 as present |
| stamp text 0 | suffix -0 | do not drop a truthy stamp |
| parent directory already exists | string, no exception | keep exist_ok success |
Build assertions with os.path.join rather than hardcoded slashes. Join rules differ across common operating systems today. A slash literal will fail on one of them.
Numbered workflow
1. Capture the call surface
List every caller argument shape before you edit. Include None, empty string, integer zero, and a text stamp. Write those shapes into the test file first.
2. Write characterization tests
Assert the current return value and the current side effect. Use a temporary directory created by the test. Point a failed parent at a file so mkdir raises.
3. Run the suite on untouched code
The first run should pass against today's helper. A failing new test means you guessed the behavior. Update the assertion to match the code, not the reverse.
4. Extract only the pure join
Move string assembly into one new pure function. Leave mkdir and the OSError handler in place. Keep the public name and the argument order unchanged.
5. Re-run the same tests
Every characterization test must still pass without edits. If one test fails, revert the extract immediately. Do not edit expectations to hide a moved side effect.
6. Stop after one extract
One pure join extract is the entire change. A type-hint sweep belongs in a later commit. A logging add also belongs in a later commit.
Tests that lock the contract
These tests are a proposal you should run locally. They use only modules from the standard library. They were not executed while preparing this article.
# proposal: unexecuted; run before you trust the assertions
import os
import tempfile
import unittest
from pathlib import Path
import report_paths
class TestBuildReportPath(unittest.TestCase):
def test_text_stamp_is_appended(self):
root = tempfile.mkdtemp(prefix='path-pin-')
got = report_paths.build_report_path(root, 'q', '01', make=False)
self.assertEqual(got, os.path.join(root, 'q-01'))
def test_zero_stamp_is_skipped(self):
root = tempfile.mkdtemp(prefix='path-pin-')
got = report_paths.build_report_path(root, 'q', 0, make=False)
self.assertEqual(got, os.path.join(root, 'q'))
def test_blank_name_uses_report(self):
root = tempfile.mkdtemp(prefix='path-pin-')
got = report_paths.build_report_path(root, '', None, make=False)
self.assertEqual(got, os.path.join(root, 'report'))
def test_failed_mkdir_returns_none(self):
root = tempfile.mkdtemp(prefix='path-pin-')
blocker = Path(root) / 'not-a-dir'
blocker.write_bytes(b'x')
parent = blocker / 'child'
got = report_paths.build_report_path(str(parent), 'q', '01', True)
self.assertIsNone(got)
def test_make_false_creates_nothing(self):
root = tempfile.mkdtemp(prefix='path-pin-')
before = set(os.listdir(root))
report_paths.build_report_path(root, 'q', '01', make=False)
self.assertEqual(set(os.listdir(root)), before)
def test_existing_parent_still_returns_string(self):
root = tempfile.mkdtemp(prefix='path-pin-')
got = report_paths.build_report_path(root, 'q', '01', make=True)
self.assertEqual(got, os.path.join(root, 'q-01'))
The failure case writes a file, then uses that file as a parent. mkdir with parents enabled must raise OSError there. The helper catches OSError and then returns None.
NotADirectoryError is a subclass of OSError in current Python. The broad handler already catches that OSError subclass. Do not narrow the except clause in this commit.
A false make flag must not create the joined path. The directory listing before and after should match. If the listing grows, the flag no longer means skip.
The existing-parent test expects a string, not None. That row guards exist_ok from a careless edit. Delete the temp directory in your real suite after each case.
The smallest safe change
# proposal: smallest extract; tests above must still pass
def join_report_path(root, name, stamp):
raw = os.path.join(str(root or ''), str(name or 'report'))
if stamp:
raw = raw + '-' + str(stamp)
return raw
def build_report_path(root, name, stamp, make=True):
path = Path(join_report_path(root, name, stamp))
if make:
try:
path.parent.mkdir(parents=True, exist_ok=True)
except OSError:
return None
return str(path)
The public function keeps its name and signature. The new function performs no mkdir and no catch. Callers of the public function still see None on failure.
A future commit can switch selected callers to the pure function. That switch is a separate behavior decision later. It does not belong beside this first extract.
Do not resolve during the extract
The public function returns a Path string, not the raw join. Wrapping with Path can collapse repeated separators. Returning the raw join from the public function would drift.
Path construction does not resolve parent-directory segments. Path.resolve can turn a relative string into an absolute path. It can also expand a symlink to another target.
Callers that compare strings will see a new value. Leave resolve for a later separate contract change. Keep the wrap inside the original function for now.
Commands to run before review
Run the characterization module before you open the diff. Then run that same module again after the extract. Both runs should report the same passing count.
python -m unittest report_paths_contract -v
If you prefer a one-file check, pass the test path directly. Keep the command in the commit message or the review note. A reviewer should be able to repeat it without extra setup.
python -m unittest discover -s tests -p 'test_report_paths*.py' -v
Do not add network calls to this suite. Do not read a developer config file inside these tests. The contract should depend only on arguments and a temp directory.
Where a free coding pass fits
You can draft the table and the tests with a coding assistant. MonkeyCode offers free model access and a free server option. Disclosure: This article was prepared as part of MonkeyCode's product outreach.
Use free model access to propose assertions from the helper you paste. Use the free server only as an isolated place to run that suite. Do not treat either option as a measured benchmark.
This article states no quota, model name, hardware shape, or retention period. Confirm those limits in current product docs before you rely on them. Availability alone is not a performance or capacity claim.
Keep the assistant outside the decision to change behavior. If a suggestion fixes the falsy zero stamp, reject it. The characterization file is the authority, and the model is only a drafter.
One practical check still belongs in this workflow. Paste the helper and ask for tests that lock current returns. Run those tests yourself on the untouched code.
A suggestion that fails on untouched code is a guess. Discard it or rewrite the assertion to match reality. Never change the helper so a guessed test can pass.
Limits of this approach
This workflow does not prove any thread safety property. Two callers can still race inside the same mkdir. It also does not prove path traversal safety.
A name with a parent-segment escape can still leave root. That risk needs a separate allow-list or a resolved-path check. Do not hide that check inside this extract.
The method fails when observed behavior is non-deterministic. Time-based stamps and random suffixes will flake under assertion. Freeze those inputs before you compare returned strings.
Network mounts and full disks can also change OSError timing. If you cannot force the failure, do not assert a specific errno. Assert only the None return you can reproduce.
Skip this method when a full contract suite already covers the helper. Also skip it during an active data-loss incident. Stop the bad write first, then characterize after containment.
Skip it when the first change must alter behavior on purpose. A deliberate new suffix rule is not a characterization task. Write the new contract, then change code to match that new contract.
Review checklist
- Tests pass on the helper before the diff.
- The diff moves join logic and nothing else.
- A caught OSError still returns None to callers.
- A false make flag still creates nothing new.
- Public names and argument order stay exactly put.
- An integer zero stamp still skips the suffix.
- No second refactor rides along in the same commit.
After the pin
Run the characterization file once more on a clean checkout. Open one commit that contains the tests plus the extract. The message should name the frozen outcomes, not a vague cleanup.
A later reader should see why None still exists. The swallowed error is recorded behavior, not an accident you forgot. Removing it requires a new contract and a new commit.
A second reader can still help after the local run. Ask a free model to hunt moved side effects. Your local test run remains the only merge gate.
Top comments (0)