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.

Sign up for GitHub

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

Jump to bottom

openshift log output on exception #492

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

openshift log output on exception #492

wants to merge 1 commit into from

Conversation

laDok8
Copy link
Contributor

@laDok8 laDok8 commented Jun 16, 2022 •
edited
Loading

issue: issue
Adding optional Openshift pod/build and event logs into test failure message.

Please make sure your PR meets the following requirements:

  • Pull Request contains a description of the changes
  • Pull Request does not include fixes for multiple issues/topics
  • Code is formatted, imports ordered, code compiles and tests are passing
  • Code is self-descriptive and/or documented

@laDok8 laDok8 requested review from mnovak and istraka June 16, 2022 10:52
@laDok8 laDok8 force-pushed the add.junitOCPLogs branch from d347254 to 2421575 Compare June 16, 2022 10:58
istraka
istraka requested changes Jun 16, 2022
View reviewed changes
junit5/src/main/java/cz/xtf/junit5/extensions/OpenShiftRecorderHandler.java Outdated Show resolved Hide resolved
junit5/src/main/java/cz/xtf/junit5/extensions/OpenShiftRecorderHandler.java Outdated Show resolved Hide resolved
junit5/src/main/java/cz/xtf/junit5/extensions/OpenShiftRecorderHandler.java Outdated Show resolved Hide resolved
junit5/src/main/java/cz/xtf/junit5/extensions/OpenShiftRecorderHandler.java Outdated Show resolved Hide resolved
junit5/src/main/java/cz/xtf/junit5/extensions/OpenShiftRecorderHandler.java Outdated Show resolved Hide resolved
junit5/src/main/java/cz/xtf/junit5/extensions/OpenShiftRecorderHandler.java Outdated Show resolved Hide resolved
junit5/src/main/java/cz/xtf/junit5/extensions/OpenShiftRecorderHandler.java Outdated Show resolved Hide resolved
junit5/src/main/java/cz/xtf/junit5/extensions/helpers/PodLogItem.java Outdated Show resolved Hide resolved
@laDok8 laDok8 force-pushed the add.junitOCPLogs branch 2 times, most recently from cbedec8 to e72e801 Compare June 20, 2022 08:10
@mchoma
Copy link
Contributor

mchoma commented Jun 20, 2022

Have you considered Service Log Stream [1] for implementing this feature? Logic would be different to current implementation. Stream to some temporary place (e.g. into String) and once test failes flush that String to System.err.

  • Advantages:

    • cronological order
    • all log there. When storing on end some containers can be gone by restarts.

  • Disadvantage are:

    • there are no events.
    • maybe chronological order is not important so much for computer pattern searching

[1] https://github.com/xtf-cz/xtf/blob/master/README.md?plain=1#L160

@laDok8
Copy link
Contributor Author

laDok8 commented Jun 23, 2022

i think having having all logs is not as important as i'm already filtering only for warning/error messages and pod causing exception should be active at that time so we should have all important info. WDYT @istraka

@laDok8 laDok8 force-pushed the add.junitOCPLogs branch from e72e801 to 40dc540 Compare June 23, 2022 11:31
@laDok8 laDok8 force-pushed the add.junitOCPLogs branch from 40dc540 to e43ece0 Compare July 7, 2022 13:15
@mnovak
Copy link
Contributor

mnovak commented Jul 12, 2022

I think there is problem when 2 test in single test case fail as in:
https://jenkins.eapqe.psi.redhat.com/job/eap-8.x-openshift-4-74/285/

Only the first one has attached logs in JUnit report but not the second one:
https://jenkins.eapqe.psi.redhat.com/job/eap-8.x-openshift-4-74/285/testReport/junit/com.redhat.xpaas.eap.smoke/SmokeTest/badAssertTest/
https://jenkins.eapqe.psi.redhat.com/job/eap-8.x-openshift-4-74/285/testReport/junit/com.redhat.xpaas.eap.smoke/SmokeTest/throwExceptionTest/

@laDok8 laDok8 force-pushed the add.junitOCPLogs branch from e43ece0 to d62d24c Compare July 14, 2022 11:04
@laDok8 laDok8 force-pushed the add.junitOCPLogs branch from d62d24c to 5e01bba Compare July 21, 2022 12:25
jbliznak
jbliznak reviewed Dec 6, 2022
View reviewed changes
junit5/src/main/java/cz/xtf/junit5/extensions/OpenShiftRecorderService.java
savePodLogs(context, getFilter(context, POD_FILTER_MASTER),
!isMasterAndBuildNamespaceSame() ? getFilter(context, POD_FILTER_BUILDS) : null, ps);
saveBuildLogs(context, getFilter(context, BUILD_FILTER_MASTER),
!isMasterAndBuildNamespaceSame() ? getFilter(context, BUILD_FILTER_MASTER) : null, ps);
Copy link
Contributor

Choose a reason for hiding this comment

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

please be aware of the same problem fixed in #521

@tommaso-borgato tommaso-borgato added this to the BACKLOG milestone Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mnovak mnovak mnovak left review comments

@istraka istraka istraka requested changes

@jbliznak jbliznak jbliznak left review comments

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
BACKLOG
Development

Successfully merging this pull request may close these issues.
6 participants
@laDok8 @mchoma @mnovak @istraka @jbliznak @tommaso-borgato