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.

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

Be test-runner friendly by only decorating stdout/stderr if status line enabled. #52

Merged
merged 1 commit into from
Aug 9, 2021

Conversation

mikepurvis
Copy link
Contributor

I have an in-progress colcon verb extension and I have a test for it which calls it via colcon_core.commands.verb_main. This works on its own and when run via nose, but fails with with an error when run via coverage, I guess because sys.stdout and sys.stderr are being overwritten in that environment to be StringIO objects which don't have the buffer attribute:

colcon.colcon_core.plugin_system: ERROR: Exception instantiating extension 'colcon_core.event_handler.status': '_io.StringIO' object has no attribute 'buffer'
Traceback (most recent call last):
  File "/home/administrator/colcon-env/lib/python3.8/site-packages/colcon_core/plugin_system.py", line 60, in _instantiate_extension
    extension_instance = extension_class()
  File "/home/administrator/colcon-env/lib/python3.8/site-packages/colcon_notification/event_handler/status.py", line 63, in __init__
    sys.stdout.buffer.write)
AttributeError: '_io.StringIO' object has no attribute 'buffer

It ends up being a bit of a cascade, because colcon actually traps this exception and carries on, with colcon-notification in a half-initialized state, so later on there are other failures from the rest of the constructor having not executed:

colcon.colcon_core.event_reactor: ERROR: Exception in event handler extension 'summary': 'StatusEventHandler' object has no attribute '_last_status_line_length'
Traceback (most recent call last):
  File "/home/administrator/colcon-env/lib/python3.8/site-packages/colcon_core/event_reactor.py", line 78, in _notify_observers
    retval = observer(event)
  File "/home/administrator/colcon-env/lib/python3.8/site-packages/colcon_output/event_handler/summary.py", line 65, in __call__
    self._print_summary()
  File "/home/administrator/colcon-env/lib/python3.8/site-packages/colcon_output/event_handler/summary.py", line 69, in _print_summary
    print()
  File "/home/administrator/colcon-env/lib/python3.8/site-packages/colcon_notification/event_handler/status.py", line 75, in wrapped_func
    self._clear_last_status_line()
  File "/home/administrator/colcon-env/lib/python3.8/site-packages/colcon_notification/event_handler/status.py", line 80, in _clear_last_status_line
    if self._last_status_line_length is not None:
AttributeError: 'StatusEventHandler' object has no attribute '_last_status_line_length

This change addresses the issue by not trying to decorate in the non-tty case.

@mikepurvis
Copy link
Contributor Author

mikepurvis commented Jun 15, 2021

Looks like the test failure is just an internal flake8 warning and is patched as of a week ago, see: python/importlib_metadata#298 (comment) but awaiting a flake8 release, as the current one is a month old.

EDIT: Oh wait... no it's not. There's some drama and a standoff between the maintainers of flake8 and importlib_metadata. Sigh.

@dirk-thomas dirk-thomas self-assigned this Aug 9, 2021
@dirk-thomas dirk-thomas added bug Something isn't working enhancement New feature or request labels Aug 9, 2021
@dirk-thomas dirk-thomas added this to the 0.2.14 milestone Aug 9, 2021
@dirk-thomas
Copy link
Member

Thanks for the patch.

I just rebased it after addressing the unrelated CI failures in #53.

@dirk-thomas dirk-thomas merged commit 74a3511 into colcon:master Aug 9, 2021
@mikepurvis mikepurvis deleted the decorate-enabled-guard branch August 9, 2021 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

None yet

2 participants