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

Automatic print command output to local stdout / stderr #134

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@jonau
Contributor

jonau commented Sep 21, 2017

To gather information about a test-failure it is sometime necessary to print the output of a command executed at the target. To reduce boilerplate in the testcases i propose to add a option to the command protocol to automatically print the output to the stdout/stderr.

jonau added some commits Sep 15, 2017

add option to print stderr and stdout when calling run-function
Signed-off-by: Johannes Nau <johannes.nau@jumo.net>
add support for run_check and barebox-driver
add option to commandprotocol

Signed-off-by: Johannes Nau <johannes.nau@jumo.net>
@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Sep 21, 2017

Codecov Report

Merging #134 into master will decrease coverage by <.1%.
The diff coverage is 52%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #134     +/-   ##
=======================================
- Coverage      50%    50%   -0.1%     
=======================================
  Files          82     82             
  Lines        4058   4070     +12     
=======================================
+ Hits         2030   2035      +5     
- Misses       2028   2035      +7
Impacted Files Coverage Δ
labgrid/protocol/commandprotocol.py 100% <100%> (ø) ⬆️
labgrid/driver/sshdriver.py 40.3% <42.8%> (+0.5%) ⬆️
labgrid/driver/bareboxdriver.py 50.6% <50%> (-0.7%) ⬇️
labgrid/driver/shelldriver.py 30.8% <50%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 24c59b8...84465cb. Read the comment docs.

codecov-io commented Sep 21, 2017

Codecov Report

Merging #134 into master will decrease coverage by <.1%.
The diff coverage is 52%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #134     +/-   ##
=======================================
- Coverage      50%    50%   -0.1%     
=======================================
  Files          82     82             
  Lines        4058   4070     +12     
=======================================
+ Hits         2030   2035      +5     
- Misses       2028   2035      +7
Impacted Files Coverage Δ
labgrid/protocol/commandprotocol.py 100% <100%> (ø) ⬆️
labgrid/driver/sshdriver.py 40.3% <42.8%> (+0.5%) ⬆️
labgrid/driver/bareboxdriver.py 50.6% <50%> (-0.7%) ⬇️
labgrid/driver/shelldriver.py 30.8% <50%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 24c59b8...84465cb. Read the comment docs.

@jonau

This comment has been minimized.

Show comment
Hide comment
@jonau

jonau Sep 22, 2017

Contributor

Maybe it is useful to also print the command together with the command promt out, so we can see the which command was used in the test-results. This would help the developers to reproduce the test failure more easily.

Contributor

jonau commented Sep 22, 2017

Maybe it is useful to also print the command together with the command promt out, so we can see the which command was used in the test-results. This would help the developers to reproduce the test failure more easily.

@Emantor

This comment has been minimized.

Show comment
Hide comment
@Emantor

Emantor Sep 22, 2017

Member

Calling pytest with -vv should already print the information for all drivers except the SSHDriver.
The step reporter should clearly show which cmd is run and what the output of the command is.
The SSHDriver is missing a result=True in the step-decorator for the run command, that should be fixed.

Member

Emantor commented Sep 22, 2017

Calling pytest with -vv should already print the information for all drivers except the SSHDriver.
The step reporter should clearly show which cmd is run and what the output of the command is.
The SSHDriver is missing a result=True in the step-decorator for the run command, that should be fixed.

@Emantor Emantor closed this Sep 22, 2017

Emantor added a commit to Emantor/labgrid that referenced this pull request Sep 22, 2017

doc/usage: document step reporter and debug logging.
After reading PR #134, I realized that the step reporter and debug logging for
pytest are undocumented. Add the documentation in the pytest plugin section.

Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>

Emantor added a commit that referenced this pull request Sep 22, 2017

doc/usage: document step reporter and debug logging.
After reading PR #134, I realized that the step reporter and debug logging for
pytest are undocumented. Add the documentation in the pytest plugin section.

Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
@jonau

This comment has been minimized.

Show comment
Hide comment
@jonau

jonau Sep 25, 2017

Contributor

I am aware of the step-reporting abilities of labgrid, which is great for debugging. I thought of the following application for the PR:
If you write a test to check for boot-errors you would do something like this:

stdout, stderr, returncode = \
    ssh.run("dmesg | grep -i 'error\|warn\|fail\|deprecated'", print=True)
assert returncode == 0
assert len(stderr) == 0
assert len(stdout) == 0

However for addressing the test failure the developer want to know what is the issue here, so you print out the result of the grep:

# only if you want to show the command
# print("dmesg | grep -i 'error\|warn\|fail\|deprecated'")
stdout, stderr, returncode = \
    ssh.run("dmesg | grep -i 'error\|warn\|fail\|deprecated'")
print("\n".join(stdout))
assert returncode == 0
assert len(stderr) == 0
assert len(stdout) == 0, "See stdout for dmesg-messages"

with my patch this would be become:

stdout, stderr, returncode = \
    ssh.run("dmesg | grep -i 'error\|warn\|fail\|deprecated'", print=True)
assert returncode == 0
assert len(stderr) == 0
assert len(stdout) == 0, "See stdout for dmesg-messages"

In this situation you can not rely on the step reporter as the output of the step reporter is not as easy to read as a direct print out and cluttered with information not relevant to the problem. Also the output goes only to terminal - which is not available in an automatic test environment - and is not shown in the test-report.

As it is easy enough to make this printouts by yourself this PR is not critically. However I thought it would be a reasonable addition.

Contributor

jonau commented Sep 25, 2017

I am aware of the step-reporting abilities of labgrid, which is great for debugging. I thought of the following application for the PR:
If you write a test to check for boot-errors you would do something like this:

stdout, stderr, returncode = \
    ssh.run("dmesg | grep -i 'error\|warn\|fail\|deprecated'", print=True)
assert returncode == 0
assert len(stderr) == 0
assert len(stdout) == 0

However for addressing the test failure the developer want to know what is the issue here, so you print out the result of the grep:

# only if you want to show the command
# print("dmesg | grep -i 'error\|warn\|fail\|deprecated'")
stdout, stderr, returncode = \
    ssh.run("dmesg | grep -i 'error\|warn\|fail\|deprecated'")
print("\n".join(stdout))
assert returncode == 0
assert len(stderr) == 0
assert len(stdout) == 0, "See stdout for dmesg-messages"

with my patch this would be become:

stdout, stderr, returncode = \
    ssh.run("dmesg | grep -i 'error\|warn\|fail\|deprecated'", print=True)
assert returncode == 0
assert len(stderr) == 0
assert len(stdout) == 0, "See stdout for dmesg-messages"

In this situation you can not rely on the step reporter as the output of the step reporter is not as easy to read as a direct print out and cluttered with information not relevant to the problem. Also the output goes only to terminal - which is not available in an automatic test environment - and is not shown in the test-report.

As it is easy enough to make this printouts by yourself this PR is not critically. However I thought it would be a reasonable addition.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment