More changes for multi DUT testcase support#2532
Merged
yxieca merged 3 commits intosonic-net:masterfrom Nov 15, 2020
Merged
Conversation
|
This pull request introduces 3 alerts when merging dc1ce30 into 3f12d64 - view on LGTM.com new alerts:
|
Collaborator
|
How did the previous test missed these? What test have been done to make sure this time the change is complete? |
yxieca
reviewed
Nov 15, 2020
|
|
||
| @pytest.fixture(scope="class", autouse=True) | ||
| def acl_rules(self, duthost, localhost, setup, acl_table, populate_vlan_arp_entries): | ||
| def acl_rules(self, duthosts, rand_one_dut_hostname, localhost, setup, acl_table, populate_vlan_arp_entries): |
Collaborator
There was a problem hiding this comment.
There is one more miss in this file. I fixed it in PR #2534. So that means the method you are using to identify these change still have issue.
Collaborator
|
We probably should merge this change if there is no negative impact on single DUT testbed and find individual issues for dualtor at individual test level. |
yxieca
approved these changes
Nov 15, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of PR
Summary: Add more changes to support MULTI DUT testing using random hostname fixture
Fixes # (issue)
Type of change
Approach
What is the motivation for this PR?
This is in continuation to merged PR #2513.
Some Pytest fixtures changes were still missing.
How did you do it?
Use two recently introduced fixtures - duthosts, rand_one_dut_hostname and retrieve duthost instance from duthosts[rand_one_dut_hostname].
How did you verify/test it?
Any platform specific information?
Supported testbed topology if it's a new test case?
Documentation