OutputWrapper class to Commands which outputs to stream object of choice

Review Request #11401 — Created Jan. 23, 2021 and updated

ryankang
RBTools
master
rbtools, students

RBTools commands were previously using print and sys.stderr
to output messages. This change abstracting outputting by using a
wrapper around a stream output object. This makes it easier to customize
output streams and suppress output. 4 wrappers (2 for standard output
unicode and byte, 2 for standard error unicode and byte) are initiated
in the Command class that is accessible to all child classes.

Ran all tests in ./tests/runtests.py rbtools.commands and passed.

Added two new tests for __init__.py. One that tests if output stream
object is set correctly for OutputWrapper and another test that makes
sure OutputWrapper passes the correct message to the output stream object

Summary
added OutputWrapper class to Commands which outputs to stream object of choice. Added test suite that checks if OutputWrapper has correct stream and that OutputWrapper passes correct message to stream object
replaced all of the prints with output wrapper and initiated wrapper in Command init
Fixed all of the Review Bot string formatting issues and fixed consistency issues with strings on multiple lines by aligning their starts
Fixed Review Bot issues on trailing white spaces, indentation, whitespace after comma, and lines over 79 characters
Added new test for OutputWrapper than ensures it passes a new line character to output stream when new_line() is called
Fixed inconsistency of the use of new_line(). Replaced self.stdout.write(n) with self.stdout.new_line()
Fixed white space error at the end of rbtools/commands/__init__.py
changed print statements in __init__.py to use output wrapper
fixed Review Bot issues where lines where over 80 characters in __init__.py
moved OutputWrapper tests from __init__.py to test_main.py and renamed test class to OutputWrapperTests
fixed indentation and misspelled words issues in test_main.py:OutputWrapper
moved OutputWrapper tests to test_main.py and imported KGB
Description From Last Updated

All the changes suggested have to do with consistency with self.stdout.new_line(). Other than that, everything else looks good to me!

amohapatraamohapatra

Your summary should be much shorter, about 70 columns. Further detail (for example, what tests were added) should be in ...

daviddavid

E501 line too long (85 > 79 characters)

reviewbotreviewbot

E501 line too long (91 > 79 characters)

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

E501 line too long (85 > 79 characters)

reviewbotreviewbot

E501 line too long (88 > 79 characters)

reviewbotreviewbot

E501 line too long (89 > 79 characters)

reviewbotreviewbot

E211 whitespace before '('

reviewbotreviewbot

E501 line too long (91 > 79 characters)

reviewbotreviewbot

E501 line too long (90 > 79 characters)

reviewbotreviewbot

E501 line too long (89 > 79 characters)

reviewbotreviewbot

E501 line too long (88 > 79 characters)

reviewbotreviewbot

E501 line too long (85 > 79 characters)

reviewbotreviewbot

E501 line too long (84 > 79 characters)

reviewbotreviewbot

E501 line too long (86 > 79 characters)

reviewbotreviewbot

E501 line too long (83 > 79 characters)

reviewbotreviewbot

E501 line too long (84 > 79 characters)

reviewbotreviewbot

E501 line too long (80 > 79 characters)

reviewbotreviewbot

E501 line too long (91 > 79 characters)

reviewbotreviewbot

E501 line too long (81 > 79 characters)

reviewbotreviewbot

E501 line too long (80 > 79 characters)

reviewbotreviewbot

E501 line too long (83 > 79 characters)

reviewbotreviewbot

E501 line too long (88 > 79 characters)

reviewbotreviewbot

E501 line too long (89 > 79 characters)

reviewbotreviewbot

E501 line too long (80 > 79 characters)

reviewbotreviewbot

W292 no newline at end of file

reviewbotreviewbot

W291 trailing whitespace

reviewbotreviewbot

E124 closing bracket does not match visual indentation

reviewbotreviewbot

E231 missing whitespace after ','

reviewbotreviewbot

E501 line too long (86 > 79 characters)

reviewbotreviewbot

W291 trailing whitespace

reviewbotreviewbot

For consistency with the other changes, should this maybe instead be self.stdout.new_line() ?

amohapatraamohapatra

Again for consistency, maybe this should be self.stdout.new_line() ?

amohapatraamohapatra

This should be self.stdout.new_line()?

amohapatraamohapatra

This should be self.stdout.new_line() ?

amohapatraamohapatra

This should also be self.stdout.new_line()

amohapatraamohapatra

This should be self.stdout.new_line()

amohapatraamohapatra

This should be self.stdout.new_line()

amohapatraamohapatra

This should be self.stdout.new_line()

amohapatraamohapatra

This should be self.stdout.new_line()

amohapatraamohapatra

This should be self.stdout.new_line()

amohapatraamohapatra

This should be self.stdout.new_line()

amohapatraamohapatra

This should be self.stdout.new_line()

amohapatraamohapatra

W292 no newline at end of file

reviewbotreviewbot

W391 blank line at end of file

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

E501 line too long (81 > 79 characters)

reviewbotreviewbot

E501 line too long (89 > 79 characters)

reviewbotreviewbot

E501 line too long (83 > 79 characters)

reviewbotreviewbot

E501 line too long (89 > 79 characters)

reviewbotreviewbot

All test cases should be in files called test_X, not the __init__.py. In this case I think you could probably ...

daviddavid

A few changes in here: The file needs a module docstring The first thing after that docstring should be from ...

daviddavid

Dedent this three spaces.

daviddavid

Typo: strea -> stream

daviddavid
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

ryankang
ryankang
ryankang
  1. 
      
  2. 
      
ryankang
Review request changed

Change Summary:

Fixing Review Bot issues and multiline string formatting

Commits:

Summary
-
added OutputWrapper class to Commands which outputs to stream object of choice. Added test suite that checks if OutputWrapper has correct stream and that OutputWrapper passes correct message to stream object
-
replaced all of the prints with output wrapper and initiated wrapper in Command init
+
added OutputWrapper class to Commands which outputs to stream object of choice. Added test suite that checks if OutputWrapper has correct stream and that OutputWrapper passes correct message to stream object
+
replaced all of the prints with output wrapper and initiated wrapper in Command init
+
Fixed all of the Review Bot string formatting issues and fixed consistency issues with strings on multiple lines by aligning their starts

Diff:

Revision 2 (+496 -344)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

ryankang
ryankang
Review request changed

Change Summary:

Added new test for Outputwrapper's new_line() that ensures output stream receives new line character

Commits:

Summary
-
added OutputWrapper class to Commands which outputs to stream object of choice. Added test suite that checks if OutputWrapper has correct stream and that OutputWrapper passes correct message to stream object
-
replaced all of the prints with output wrapper and initiated wrapper in Command init
-
Fixed all of the Review Bot string formatting issues and fixed consistency issues with strings on multiple lines by aligning their starts
-
Fixed Review Bot issues on trailing white spaces, indentation, whitespace after comma, and lines over 79 characters
+
added OutputWrapper class to Commands which outputs to stream object of choice. Added test suite that checks if OutputWrapper has correct stream and that OutputWrapper passes correct message to stream object
+
replaced all of the prints with output wrapper and initiated wrapper in Command init
+
Fixed all of the Review Bot string formatting issues and fixed consistency issues with strings on multiple lines by aligning their starts
+
Fixed Review Bot issues on trailing white spaces, indentation, whitespace after comma, and lines over 79 characters
+
Added new test for OutputWrapper than ensures it passes a new line character to output stream when new_line() is called

Diff:

Revision 4 (+527 -355)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

amohapatra
  1. 
      
  2. rbtools/commands/patch.py (Diff revision 4)
     
     

    For consistency with the other changes, should this maybe instead be self.stdout.new_line() ?

  3. rbtools/commands/post.py (Diff revision 4)
     
     

    Again for consistency, maybe this should be self.stdout.new_line() ?

  4. rbtools/commands/setup_repo.py (Diff revision 4)
     
     

    This should be self.stdout.new_line()?

  5. rbtools/commands/setup_repo.py (Diff revision 4)
     
     

    This should be self.stdout.new_line() ?

  6. rbtools/commands/setup_repo.py (Diff revision 4)
     
     

    This should also be self.stdout.new_line()

  7. rbtools/commands/setup_repo.py (Diff revision 4)
     
     

    This should be self.stdout.new_line()

  8. rbtools/commands/setup_repo.py (Diff revision 4)
     
     

    This should be self.stdout.new_line()

  9. rbtools/commands/setup_repo.py (Diff revision 4)
     
     

    This should be self.stdout.new_line()

  10. rbtools/commands/setup_repo.py (Diff revision 4)
     
     

    This should be self.stdout.new_line()

  11. rbtools/commands/setup_repo.py (Diff revision 4)
     
     

    This should be self.stdout.new_line()

  12. rbtools/commands/setup_repo.py (Diff revision 4)
     
     

    This should be self.stdout.new_line()

  13. rbtools/commands/setup_repo.py (Diff revision 4)
     
     

    This should be self.stdout.new_line()

  14. 
      
amohapatra
  1. 
      
  2. All the changes suggested have to do with consistency with self.stdout.new_line(). Other than that, everything else looks good to me!

  3. 
      
ryankang
Review request changed

Change Summary:

Replaced self.stdout.write("\n") with self.stdout.new_line()

Commits:

Summary
-
added OutputWrapper class to Commands which outputs to stream object of choice. Added test suite that checks if OutputWrapper has correct stream and that OutputWrapper passes correct message to stream object
-
replaced all of the prints with output wrapper and initiated wrapper in Command init
-
Fixed all of the Review Bot string formatting issues and fixed consistency issues with strings on multiple lines by aligning their starts
-
Fixed Review Bot issues on trailing white spaces, indentation, whitespace after comma, and lines over 79 characters
-
Added new test for OutputWrapper than ensures it passes a new line character to output stream when new_line() is called
+
added OutputWrapper class to Commands which outputs to stream object of choice. Added test suite that checks if OutputWrapper has correct stream and that OutputWrapper passes correct message to stream object
+
replaced all of the prints with output wrapper and initiated wrapper in Command init
+
Fixed all of the Review Bot string formatting issues and fixed consistency issues with strings on multiple lines by aligning their starts
+
Fixed Review Bot issues on trailing white spaces, indentation, whitespace after comma, and lines over 79 characters
+
Added new test for OutputWrapper than ensures it passes a new line character to output stream when new_line() is called
+
Fixed inconsistency of the use of new_line(). Replaced self.stdout.write(n) with self.stdout.new_line()

Diff:

Revision 5 (+543 -369)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

ryankang
ryankang
ryankang
Review request changed

Commits:

Summary
-
added OutputWrapper class to Commands which outputs to stream object of choice. Added test suite that checks if OutputWrapper has correct stream and that OutputWrapper passes correct message to stream object
-
replaced all of the prints with output wrapper and initiated wrapper in Command init
-
Fixed all of the Review Bot string formatting issues and fixed consistency issues with strings on multiple lines by aligning their starts
-
Fixed Review Bot issues on trailing white spaces, indentation, whitespace after comma, and lines over 79 characters
-
Added new test for OutputWrapper than ensures it passes a new line character to output stream when new_line() is called
-
Fixed inconsistency of the use of new_line(). Replaced self.stdout.write(n) with self.stdout.new_line()
-
Fixed white space error at the end of rbtools/commands/__init__.py
+
added OutputWrapper class to Commands which outputs to stream object of choice. Added test suite that checks if OutputWrapper has correct stream and that OutputWrapper passes correct message to stream object
+
replaced all of the prints with output wrapper and initiated wrapper in Command init
+
Fixed all of the Review Bot string formatting issues and fixed consistency issues with strings on multiple lines by aligning their starts
+
Fixed Review Bot issues on trailing white spaces, indentation, whitespace after comma, and lines over 79 characters
+
Added new test for OutputWrapper than ensures it passes a new line character to output stream when new_line() is called
+
Fixed inconsistency of the use of new_line(). Replaced self.stdout.write(n) with self.stdout.new_line()
+
Fixed white space error at the end of rbtools/commands/__init__.py
+
changed print statements in __init__.py to use output wrapper

Diff:

Revision 7 (+570 -398)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

ryankang
david
  1. 
      
  2. rbtools/commands/tests/__init__.py (Diff revision 8)
     
     

    All test cases should be in files called test_X, not the __init__.py. In this case I think you could probably add your new test class to the existing test_main.py file.

    This class should also probably be called OutputWrapperTests.

  3. rbtools/commands/tests/__init__.py (Diff revision 8)
     
     
     
     
     
     

    A few changes in here:

    1. The file needs a module docstring
    2. The first thing after that docstring should be from __future__ import unicode_literals
    3. Imports should be structured in three groups: standard library, third-party libraries, and then rbtools. In each group we alphabetize the imports. So these should look like:

    import sys
    
    from kgb import SpyAgency
    
    from rbtools.commands import OutputWrapper
    from rbtools.utils.testbase import RBTestBase
    
  4. rbtools/commands/tests/__init__.py (Diff revision 8)
     
     

    Dedent this three spaces.

  5. rbtools/commands/tests/__init__.py (Diff revision 8)
     
     

    Typo: strea -> stream

  6. 
      
david
  1. 
      
  2. Your summary should be much shorter, about 70 columns. Further detail (for example, what tests were added) should be in the description.

  3. 
      
ryankang
ryankang
Review request changed

Commits:

Summary
-
added OutputWrapper class to Commands which outputs to stream object of choice. Added test suite that checks if OutputWrapper has correct stream and that OutputWrapper passes correct message to stream object
-
replaced all of the prints with output wrapper and initiated wrapper in Command init
-
Fixed all of the Review Bot string formatting issues and fixed consistency issues with strings on multiple lines by aligning their starts
-
Fixed Review Bot issues on trailing white spaces, indentation, whitespace after comma, and lines over 79 characters
-
Added new test for OutputWrapper than ensures it passes a new line character to output stream when new_line() is called
-
Fixed inconsistency of the use of new_line(). Replaced self.stdout.write(n) with self.stdout.new_line()
-
Fixed white space error at the end of rbtools/commands/__init__.py
-
changed print statements in __init__.py to use output wrapper
-
fixed Review Bot issues where lines where over 80 characters in __init__.py
-
moved OutputWrapper tests from __init__.py to test_main.py and renamed test class to OutputWrapperTests
-
fixed indentation and misspelled words issues in test_main.py:OutputWrapper
+
added OutputWrapper class to Commands which outputs to stream object of choice. Added test suite that checks if OutputWrapper has correct stream and that OutputWrapper passes correct message to stream object
+
replaced all of the prints with output wrapper and initiated wrapper in Command init
+
Fixed all of the Review Bot string formatting issues and fixed consistency issues with strings on multiple lines by aligning their starts
+
Fixed Review Bot issues on trailing white spaces, indentation, whitespace after comma, and lines over 79 characters
+
Added new test for OutputWrapper than ensures it passes a new line character to output stream when new_line() is called
+
Fixed inconsistency of the use of new_line(). Replaced self.stdout.write(n) with self.stdout.new_line()
+
Fixed white space error at the end of rbtools/commands/__init__.py
+
changed print statements in __init__.py to use output wrapper
+
fixed Review Bot issues where lines where over 80 characters in __init__.py
+
moved OutputWrapper tests from __init__.py to test_main.py and renamed test class to OutputWrapperTests
+
fixed indentation and misspelled words issues in test_main.py:OutputWrapper
+
moved OutputWrapper tests to test_main.py and imported KGB

Diff:

Revision 10 (+617 -449)

Show changes

Checks run (2 succeeded)

flake8 passed.
JSHint passed.
Loading...