-
Notifications
You must be signed in to change notification settings - Fork 110
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 rex connect by ip #4648
test rex connect by ip #4648
Conversation
@pondrejk, thanks for your PR! By analyzing the history of the files in this pull request, we identified @JacobCallahan, @oshtaier and @abalakh to be potential reviewers. |
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.
ACK
d4e1741
to
4dbf84f
Compare
I realized the UI part needs some more work so I decided to move it to a separate commit, test results for the cli part:
|
|
||
:expectedresults: Verify the job was successfully ran against the host | ||
""" | ||
Settings.set({ |
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.
same comment here about try...finally
block
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.
@oshtaier addressed, ok with this form?
4dbf84f
to
679cbf4
Compare
679cbf4
to
39977aa
Compare
Hello, after some research and discussion with @rplevka, I decided to take a different approach to these tests (using per-host setting instead of global setting). Therefore I put do_not_merge. Also this can help us to get more rex tests running in automation until the problems from #4578 are resolved |
5f799ab
to
db5ced6
Compare
Codecov Report
@@ Coverage Diff @@
## master #4648 +/- ##
=======================================
Coverage 41.21% 41.21%
=======================================
Files 88 88
Lines 7163 7163
=======================================
Hits 2952 2952
Misses 4211 4211 Continue to review full report at Codecov.
|
db5ced6
to
7b4c8d1
Compare
@renzon @oshtaier ready for review again, I enable the remote_execution_connect_by_ip per host, I added more tests -- these are by_ip version of existing tests. Tests:
Also added a skip for two tests due to https://bugzilla.redhat.com/show_bug.cgi?id=1451675 |
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.
ACK
7b4c8d1
to
c26c607
Compare
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.
ACK
Related to #4624 tests TBA