-
Notifications
You must be signed in to change notification settings - Fork 196
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
USHIFT-2431: Add junit integration to CNCF tests #3108
Conversation
@ggiguash: This pull request references USHIFT-2431 which is a valid jira issue. In response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Skipping CI for Draft Pull Request. |
/test ? |
@ggiguash: The following commands are available to trigger required jobs:
The following commands are available to trigger optional jobs:
Use
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/test metal-periodic-test |
start_junit | ||
trap "close_junit" EXIT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The framework creates and closes the file, so you don't want these 2 lines. Calling start_junit again will overwrite any existing content and having the trap will cause the file to be closed twice so the syntax is not correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dhellmann , it looks like the tests do not work without this change.
The reason is because the junit is initialized in run_tests
function, which is never called for CNCF.
I have to return the code back
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is the plugin interface used by run_tests
. If run_tests
is not called, then these lines wouldn't do anything, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at scenario.sh
, running tests is a separate action and it calls action_run
function. This function is different from action_create
because it does not directly call start_junit
.
The latter is only called inside run_tests
function (RF invocation), which is never used in the CNCF test because it's not written using RF.
One possible solution to this problem would be to move the following block to action_run
instead of run_tests
.
start_junit
trap "close_junit; sos_report" EXIT
Then, it will always be called regardless of what's executed in the scenario run.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I uploaded this implementation - let me know what you think.
/assign @dhellmann @pacevedom |
/test metal-periodic-test microshift-metal-tests |
/lgtm I like the change to move the junit setup into the action function. It's much more consistent. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dhellmann, ggiguash The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@ggiguash: all tests passed! Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
No description provided.