Skip to content

Fix remaining stubtest errors - #791

Merged
amoffat merged 1 commit into
amoffat:developfrom
jorenham:typing/stubtest-fixes
Jul 23, 2026
Merged

amoffat merged 1 commit into
amoffat:developfrom
jorenham:typing/stubtest-fixes

Conversation

@jorenham

Copy link
Copy Markdown
Contributor

As promised in #790, here's the last PR in the stubtest series, bringing the stubtest errors down to...

(drumroll please)

... 0 🎉!!

$ stubtest sh
Success: no issues found in 1 module

Oh, one thing I wasn't really sure about is the removal of SignalException_SIGUSR1 and SignalException_SIGUSR2. Stubtest wanted me to, so I simply did. But I was left wondering whether maybe it was a system-dependent thing that's should stay?

Anyway, aftyer this, I plan on working on adding stubtest to the CI pipeline so we can ensure that the stubs stay in sync with the runtime API.

@amoffat

amoffat commented Jul 23, 2026

Copy link
Copy Markdown
Owner

Awesome!! 🎉🎉🎉

We should leave SIGUSR1 and SIGUSR2. It's strange that stubtest was complaining about those, since they should be like the other signals. Otherwise this looks great 🙌

@jorenham

jorenham commented Jul 23, 2026 •

Copy link
Copy Markdown
Contributor Author

Hmm, I think that's a runtime bug then:

>>> import sh
>>> sh.SignalException_SIGKILL
<class 'sh.SignalException_SIGKILL'>
>>> sh.SignalException_SIGUSR1
Traceback (most recent call last):
  File "/home/joren/Workspace/sh/src/sh/__init__.py", line 390, in get_exc_from_name
    return rc_exc_cache[name]
           ~~~~~~~~~~~~^^^^^^
KeyError: 'SignalException_SIGUSR1'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/joren/Workspace/sh/src/sh/__init__.py", line 399, in get_exc_from_name
    rc = -int(rc_or_sig_name)
          ~~~^^^^^^^^^^^^^^^^
ValueError: invalid literal for int() with base 10: 'SIGUSR'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
    sh.SignalException_SIGUSR1
  File "/home/joren/Workspace/sh/src/sh/__init__.py", line 3742, in __getattr__
    return self.__env[name]
           ~~~~~~~~~~^^^^^^
  File "/home/joren/Workspace/sh/src/sh/__init__.py", line 3487, in __getitem__
    exc = get_exc_from_name(k)
  File "/home/joren/Workspace/sh/src/sh/__init__.py", line 401, in get_exc_from_name
    rc = -getattr(signal, rc_or_sig_name)
          ~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: module 'signal' has no attribute 'SIGUSR'. Did you mean: 'SIGUSR1'?

This seems to be caused by the regex at line 369:

rc_exc_regex = re.compile(r"(ErrorReturnCode|SignalException)_((\d+)|SIG[a-zA-Z]+)")

The SIG[a-zA-Z]+ branch has no digits, and since it's re.match (prefix match, not fullmatch), "SignalException_SIGUSR1" matches with the 2nd group, i.e. "SIGUSR", dropping the trailing digit. But signal.SIGUSR isn't a thing, so that leads to the AttributeError above.

Copilot AI added a commit that referenced this pull request Jul 23, 2026
…ion_SIGUSR2

The regex `rc_exc_regex` used `SIG[a-zA-Z]+` which does not match digit
suffixes. Signal names like SIGUSR1/SIGUSR2 were truncated to "SIGUSR",
causing `getattr(signal, 'SIGUSR')` to raise an AttributeError.

Extend the character class to `SIG[a-zA-Z0-9]+` so the full name is
captured, and add a regression test. References #791.
@amoffat

amoffat commented Jul 23, 2026

Copy link
Copy Markdown
Owner

Ok fixed, you should be good to proceed

@jorenham
jorenham force-pushed the typing/stubtest-fixes branch from 46107ba to a0b2d5d Compare July 23, 2026 20:58
@amoffat
amoffat merged commit 04f6541 into amoffat:develop Jul 23, 2026
23 checks passed
@jorenham
jorenham deleted the typing/stubtest-fixes branch July 23, 2026 21:28
@jorenham

Copy link
Copy Markdown
Contributor Author

Thanks, I really appreciate your quick responses :)

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