search

avocado-framework / avocado

Avocado is a set of tools and libraries to help with automated testing. One can call it a test framework with benefits. Native tests are written in Python and they follow the unittest pattern, but any executable can serve as a test.
https://avocado-framework.github.io/
Other
342 stars 340 forks source link

Avoiding problems like "'StreamToQueue' object has no attribute 'isatty'" downstream #5416

Open pevogam opened 2 years ago

pevogam commented 2 years ago

Discussed in https://github.com/avocado-framework/avocado/discussions/4953

Originally posted by **pevogam** September 17, 2021 As Avocado's runners are replacing the stdout and stderr of the tests with a custom `StreamToQueue` class, it might be a better idea if this `StreamToQueue` inherits from or has behavior that is more compatible with python's default stdout. A test script or test utility downstream might for instance do `if stdout.isatty` that will result in breakage because it might not necessarily assume Avocado has swapped the output channels (e.g. if the code is used in more general scopes beyond avocado). Do you think we should make the `StreamToQueue` objects more compatible with stdout objects or do you think there is a problem in the argument here?
pevogam commented 2 years ago

@clebergnu @beraldoleal @richtja I just had another trouble like this: an imported library that is used in the tests needed to use stderr in

[stdlog] 2022-06-28 11:08:07,666 avocado.test ERROR|   File "/usr/lib/python3.9/site-packages/graphviz/_compat.py", line 61, in stderr_write_bytes
[stdlog] 2022-06-28 11:08:07,666 avocado.test ERROR|     encoding = sys.stderr.encoding or sys.getdefaultencoding()
[stdlog] 2022-06-28 11:08:07,666 avocado.test ERROR| AttributeError: 'StreamToQueue' object has no attribute 'encoding'

where in a regular sys.stderr import we will have it:

[pevogam@drogon avocado-i2n]$ python3
Python 3.9.13 (main, May 18 2022, 00:00:00) 
[GCC 11.3.1 20220421 (Red Hat 11.3.1-2)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> sys.stderr.encoding
'utf-8'

To keep it short, if we implicitly replace the standard streams we should make sure we provide maximum compatibility because literally any external library that is not aware it is run from an avocado test might break in such instances.

clebergnu commented 2 years ago

@pevogam I think you are on the right track. This was an almost unsolvable problem with the legacy runner, because of its forking/execing model.

Now with nrunner, we are better positioned to remove the existence of StreamToQueue. The actual runners (say avocado-runner-exec-test) already capture the stderr/stdout of the executed process.

Let me know if this makes sense to you.

pevogam commented 2 years ago

@pevogam I think you are on the right track. This was an almost unsolvable problem with the legacy runner, because of its forking/execing model.

Now with nrunner, we are better positioned to remove the existence of StreamToQueue. The actual runners (say avocado-runner-exec-test) already capture the stderr/stdout of the executed process.

Let me know if this makes sense to you.

Removal also sounds good and adds the reduced maintenance costs on top so it makes complete sense. Should I rephrase this issue as "remove StreamToQueue" or would you like to create a separate issue about this concrete plan?