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

test.sit-test-cases: Redirect test output to a file #67

Merged
merged 1 commit into from
Nov 23, 2023

Conversation

anoopcs9
Copy link
Collaborator

@anoopcs9 anoopcs9 commented Nov 21, 2023

Instead of cluttering the jenkins job console output with detailed test results we could redirect everything to a separate file and make it available as artifacts for each job. Simplest way to achieve this redirection is to make use of &>(see section 3.6.4 bash manual) to include both STDOUT and STDERR. Location of such an output file is chosen to be under /var/log so that it also gets collected as part of statedump process.

anoopcs9 added a commit to anoopcs9/sit-environment that referenced this pull request Nov 21, 2023
* smbtorture tests have been configured[1] to print the results
  irrespective of the outcome.
* Test output is now redirected to a separate file[2], additional
  debug lines from ansible are no longer required.

[1] samba-in-kubernetes/sit-test-cases#44
[2] samba-in-kubernetes#67

Signed-off-by: Anoop C S <anoopcs@cryptolab.net>
anoopcs9 added a commit to anoopcs9/sit-environment that referenced this pull request Nov 21, 2023
* smbtorture tests have been configured[1] to print the results
  irrespective of the outcome.
* Test output is now redirected to a separate file[2], additional
  debug lines from ansible are no longer required.

[1] samba-in-kubernetes/sit-test-cases#44
[2] samba-in-kubernetes#67

Signed-off-by: Anoop C S <anoopcs@cryptolab.net>
xhernandez
xhernandez previously approved these changes Nov 21, 2023
@spuiuk
Copy link
Collaborator

spuiuk commented Nov 21, 2023

At the moment you only capture the stdout. Maybe we should capture stderr as well with 2>&1?

@anoopcs9
Copy link
Collaborator Author

At the moment you only capture the stdout.

Correct.

Maybe we should capture stderr as well with 2>&1?

But what are we missing? Everything we used to see previously should be redirected to the mentioned file. If there's something that we specifically write to stderr and not stdout, we are already(currently) missing those lines. See test.out.txt as an example for a failure case.

anoopcs9 added a commit to anoopcs9/sit-environment that referenced this pull request Nov 22, 2023
* smbtorture tests have been configured[1] to print the results
  irrespective of the outcome.
* Test output is now redirected to a separate file[2], additional
  debug lines from ansible are no longer required.

Changes to Makefile is basically the revert of 15b8f61.

[1] samba-in-kubernetes/sit-test-cases#44
[2] samba-in-kubernetes#67

Signed-off-by: Anoop C S <anoopcs@cryptolab.net>
@anoopcs9
Copy link
Collaborator Author

Maybe we should capture stderr as well with 2>&1?

But what are we missing? Everything we used to see previously should be redirected to the mentioned file. If there's something that we specifically write to stderr and not stdout, we are already(currently) missing those lines. See test.out.txt as an example for a failure case.

Here's how the console output will look like:

TASK [test.sit-test-cases : Run tests] *****************************************
Wednesday 22 November 2023 05:58:01 +0000 (0:00:00.058) 0:00:05.373 ****
fatal: [client0]: FAILED! => {"changed": true, "cmd": "make test > /var/log/test.out", "delta": "0:12:21.031685", "end": "2023-11-22 06:10:22.800463", "msg": "non-zero return code", "rc": 2, "start": "2023-11-22 05:58:01.768778", "stderr": "make: *** [Makefile:15: test] Error 1", "stderr_lines": ["make: *** [Makefile:15: test] Error 1"], "stdout": "", "stdout_lines": []}

make: *** [Makefile:15: test] Error 1 is the one that might get missed from test output file. If required I can include that as well.

spuiuk
spuiuk previously approved these changes Nov 22, 2023
Copy link
Collaborator

@spuiuk spuiuk left a comment

Choose a reason for hiding this comment

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

ACK

This is fine for now. If we want to consider capturing STDERR later, we can revisit this.

obnoxxx
obnoxxx previously approved these changes Nov 22, 2023
Copy link
Collaborator

@obnoxxx obnoxxx left a comment

Choose a reason for hiding this comment

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

the changes look good to me, but I generally prefer to have a bit more of a justification and explanation of the changes in the commit message and in the PR description. I. E.:

  • why is it needed? (what problem does it fix?)
  • how is it done? (e.g. mention the output file name in this case.)
  • sometimes: why is it done this way?

Approving this one now, but I might request such details for future PRs ...

@anoopcs9
Copy link
Collaborator Author

ACK

This is fine for now. If we want to consider capturing STDERR later, we can revisit this.

I've then decided to include STDERR too which seems to be a small addition.

the changes look good to me, but I generally prefer to have a bit more of a justification and explanation of the changes in the commit message and in the PR description. I. E.:

Thanks, let me try to include more details with the next version.

Instead of cluttering the jenkins job console output with detailed test
results we could redirect everything to a separate file and make it
available as artifacts for each job. Simplest way to achieve this
redirection is to make use of '&>'[1] to include both STDOUT and STDERR.
Location of such an output file is chosen to be under /var/log so that
it also gets collected as part of statedump process.

[1] https://www.gnu.org/software/bash/manual/html_node/Redirections.html(3.6.4)

Signed-off-by: Anoop C S <anoopcs@cryptolab.net>
anoopcs9 added a commit to anoopcs9/sit-environment that referenced this pull request Nov 22, 2023
* smbtorture tests have been configured[1] to print the results
  irrespective of the outcome.
* Test output is now redirected to a separate file[2], additional
  debug lines from ansible are no longer required.

Previous change from 15b8f61 to
configure ANSIBLE_STDOUT_CALLBACK option can also be avoided as
entire results are now available in a separate file.

[1] samba-in-kubernetes/sit-test-cases#44
[2] samba-in-kubernetes#67

Signed-off-by: Anoop C S <anoopcs@cryptolab.net>
Copy link
Collaborator

@spuiuk spuiuk left a comment

Choose a reason for hiding this comment

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

ACK

@anoopcs9
Copy link
Collaborator Author

anoopcs9 commented Nov 22, 2023

/retest centos-ci/cephfs

@spuiuk
Copy link
Collaborator

spuiuk commented Nov 22, 2023

/test centos-ci/cephfs

@spuiuk
Copy link
Collaborator

spuiuk commented Nov 22, 2023

/retest centos-ci/cephfs

@anoopcs9 anoopcs9 merged commit 082eb92 into samba-in-kubernetes:main Nov 23, 2023
6 checks passed
@anoopcs9 anoopcs9 deleted the redirect-test-out-file branch November 23, 2023 06:56
anoopcs9 added a commit to anoopcs9/sit-environment that referenced this pull request Nov 23, 2023
* smbtorture tests have been configured[1] to print the results
  irrespective of the outcome.
* Test output is now redirected to a separate file[2], additional
  debug lines from ansible are no longer required.

Previous change from 15b8f61 to
configure ANSIBLE_STDOUT_CALLBACK option can also be avoided as
entire results are now available in a separate file.

[1] samba-in-kubernetes/sit-test-cases#44
[2] samba-in-kubernetes#67

Signed-off-by: Anoop C S <anoopcs@cryptolab.net>
anoopcs9 added a commit that referenced this pull request Nov 23, 2023
* smbtorture tests have been configured[1] to print the results
  irrespective of the outcome.
* Test output is now redirected to a separate file[2], additional
  debug lines from ansible are no longer required.

Previous change from 15b8f61 to
configure ANSIBLE_STDOUT_CALLBACK option can also be avoided as
entire results are now available in a separate file.

[1] samba-in-kubernetes/sit-test-cases#44
[2] #67

Signed-off-by: Anoop C S <anoopcs@cryptolab.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants