Sweeping changes for Dual ToR - use duthosts and rand_one_dut_hostname

[vaibhavhd]

Description of PR

Summary: Common, repetitive changes to make testcases compatible for Dual ToR testbed.

Type of change

• Bug fix
• Testbed and Framework(new/improvement)
• Test case(new/improvement)

Approach

What is the motivation for this PR?

Testing Dual ToR testbed. The changes included in this PR are common changes done for each testcase. To avoid repetitive conflicts and changes in several PRs, this PR does it all at once.

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].

Also, modified the scope of rand_one_dut_hostname fixture to module.
This is needed in the cases when test_setup fixture is parametrized for different params and rand_one_dut_hostname fixture might return random (different) DUT for each param.
Module scope ensures that all the testcases in one module work on same DUT in the dual ToR testbed.
Additionally, many fixtures in testcases have module scope, and using function scope fixture throws an errors:
ScopeMismatch: You tried to access the 'function' scoped fixture 'rand_one_dut_hostname' with a 'module' scoped request object, involved factories

How did you verify/test it?

Executed a few testcases successfully - test_lldp, test_iface_namingmode (still testing a few more).

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

[lgtm-com]

This pull request introduces 145 alerts when merging a309551 into 1317643 - view on LGTM.com

new alerts:

• 139 for Unused local variable
• 4 for Wrong number of arguments in a call
• 1 for Unnecessary pass
• 1 for Wrong number of arguments in a class instantiation

[lgtm-com]

This pull request introduces 137 alerts when merging d189ece into 1317643 - view on LGTM.com

new alerts:

• 137 for Unused local variable

[vaibhavhd]

Retest vsimage please

[neethajohn]

@yxieca, was the plan to have all testcases run against the same dut? I was under the assumption that picking diff duts per test case would enable us to introduce some logic after collection to distribute load across duts

[vaibhavhd]

@yxieca, was the plan to have all testcases run against the same dut? I was under the assumption that picking diff duts per test case would enable us to introduce some logic after collection to distribute load across duts

Help me understand why do you think that the all testcases will run against the same dut? The rand_one_dut_hostname fixture selects random DUT for every test module:

        dut_hostnames = generate_params_dut_hostname(metafunc)
        if len(dut_hostnames) > 1:
            dut_hostnames = random.sample(dut_hostnames, 1)
        metafunc.parametrize("rand_one_dut_hostname", dut_hostnames, scope='module')

[neethajohn]

@vaibhavhd, all good. was assuming its session scoped

[lgtm-com]

This pull request introduces 1 alert when merging 6ef2058 into 1317643 - view on LGTM.com

new alerts:

• 1 for Unused local variable

[lgtm-com]

This pull request introduces 1 alert when merging 339cd5f into ffb0902 - view on LGTM.com

new alerts:

• 1 for Unused local variable

[yxieca]

We also want to revert change under platform_tests/api/, right?

[vaibhavhd]

We also want to revert change under platform_tests/api/, right?

Yes, that change is already added. We are not touching platform_tests/api/ Do you see something unintended in the diff?