Skip to content
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

Jump to bottom

Fix bug introduced in 'genpy.py' by commit adf9cfb. #2461

Merged
merged 1 commit into from
Jan 16, 2025
Merged

Fix bug introduced in 'genpy.py' by commit adf9cfb. #2461

merged 1 commit into from
Jan 16, 2025

Conversation

geppi
Copy link
Contributor

@geppi geppi commented Jan 15, 2025

Commit adf9cfb introduced a bug into genpy.py that causes an exception when running gencache.EnsureModule on a TypeLibrary that defines COM Records:

File ".....\site-packages\win32com\client\genpy.py", line 1152, in do_generate
    print(f"\t{record_str}", file=stream)
UnboundLocalError: cannot access local variable 'record_str' where it is not associated with a value

I'm mildly surprised that this commit passed the tests.

@geppi geppi changed the title Fix bug introduced in 'genpy.py' by commit adf9cfb89652050936c7f742179fe1cac48cc933. Fix bug introduced in 'genpy.py' by commit adf9cfb. Jan 15, 2025
@geppi geppi mentioned this pull request Jan 15, 2025
Merged
mhammond
mhammond approved these changes Jan 15, 2025
View reviewed changes
Copy link
Owner

@mhammond mhammond left a comment

Choose a reason for hiding this comment

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

Thanks! I also don't understand how CI was passing!

@Avasam
Copy link
Collaborator

Avasam commented Jan 15, 2025 •
edited
Loading

Thanks!

I'm mildly surprised that this commit passed the tests.

I guess there's no runtime tests on the CI for when record.clsid != pythoncom.IID_NULL

Statically this would be caught by https://github.com/microsoft/pyright/blob/main/docs/configuration.md#reportPossiblyUnboundVariable and https://mypy.readthedocs.io/en/stable/error_code_list2.html#warn-about-variables-that-are-defined-only-in-some-execution-paths-possibly-undefined

@geppi
Copy link
Contributor Author

geppi commented Jan 16, 2025 •
edited
Loading

Indeed testPyComTest.py does catch this but:

  1. It is not part of the CI pipeline as far as I understand.
  2. When I ran testPyComTest.py locally to check Implementation of 'com_record' as a subclassable Python type. #2437, I did this before merging in the latest changes of the main branch which would have included the culprit.
  3. You have to assure that the module for PyCOMTestLib does not already exist in the gen_py folder.

@mhammond
Copy link
Owner

Great, thanks for checking!

@mhammond mhammond merged commit e2a574c into mhammond:main Jan 16, 2025
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mhammond mhammond mhammond approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@geppi @Avasam @mhammond