Skip to content

Bugfest: obscure edge cases - #59

Open
thatch wants to merge 10 commits into
mainfrom
thatch/obscure-edge-cases
Open

thatch wants to merge 10 commits into
mainfrom
thatch/obscure-edge-cases

Conversation

@thatch

@thatch thatch commented Sep 10, 2026

Copy link
Copy Markdown
Member

No description provided.

thatch and others added 10 commits April 28, 2026 19:21
Pass ignore_errors=False to shutil.rmtree so OS errors propagate to
the try/except in delete(), allowing failed directory deletions to
correctly return -1 instead of 1.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When Anchored.short (or any adapter) transforms a path to "",
the empty string was still appended to result and joined, producing
extra delimiters in the output. Add an early `continue` to skip
empty strings after adapter transformation.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
In run()'s dryrun early-return, result.error was initialized to "" only
when stdout was captured (duplicate condition), never when stderr was
captured. Change the second condition to check stderr.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
COLUMNS can make short() truncate expected full strings and cause spurious failures.
@thatch
thatch requested a review from zsimic September 10, 2026 01:48

@zsimic zsimic left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice! Quite a few bugs! Some are unclear why they were bugs, but willing to trust :) Looks likely legit for those too

Comment thread src/runez/file.py

else:
shutil.rmtree(path, ignore_errors=not fatal)
shutil.rmtree(path, ignore_errors=False)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What was wrong with not fatal?

Comment thread src/runez/program.py

if stdout is not None:
if stderr is not None:
result.error = ""

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I need to change this so result.error = "" all the time... there's not much value actually to have this as str | None, but this kind of ok (it should really be if stdout is not None or stderr is not None or is_passthrough... (it's the gist of it -> not None if there was anything captured)

Comment thread src/runez/system.py
def canonical_platform(name):
name = name and name.lower()
if not name:
return name

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

OK but seems not needed...

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.

2 participants