-
Notifications
You must be signed in to change notification settings - Fork 32
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
[e2e] add support bundle test cases #1408
base: main
Are you sure you want to change the base?
Conversation
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.
LGTM.
Can we take this time and handle few more things in supportbundle tests as below
- The test_longfile_exists doesn't check all the log files, can we include all the log files checks?
- I think we can skip the test_delete for supportbundle. It would be good if we have supportbundle after tests run.
f"{fails}" | ||
) | ||
|
||
support_bundle_state.fio.seek(0) |
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.
Is this necessary?
If so, perhaps better move to L#121
?
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.
It is necessary when following test cases going to read the fio
again. and yeah move to L121 would be good.
|
If you can't add test here as you don't have idea, could you help create a test ticket to cover it.
Yes |
so yeah, please create another ticket for your request, not ASK them in PRs. |
I don't want to fight in public space, but I FREQUENTLY FEEL uncomfortable when a PR is resolving some issues, but it be asked to stack up to fix other problems. |
Changes
test_secret_file_exists
ref to [e2e] [FEATURE] Collect plan secrets in support bundles #603test_hardware_info_exists
ref to [e2e] [FEATURE] add hwinfo to supportbundle #569