Unify the importing to avoid exception not caught issue

[wangxin]

Description of PR

Summary:
Fixes # (issue)

The sonic-mgmt/tests folder was added to sys.path so that the scripts
and libs can use format from common.xx import xx to import from
the common folder. By this way, objects may be identified as 'tests.common.xx'
or 'common.xx' under different context. This caused some side effects
like RunAnsibleModuleFail exception cannot be caught. The raised
exception is identified as tests.common.errors.RunAnsibleModuleFail.
But the script tries to catch common.errors.RunAnsibleModuleFail and
fail to catch the exception.

This PR removes the code for adding sonic-mgmt/tests/ to sys.path and
updated the related importing to format like from tests.common.xx import xx.
When we use pytest to run scripts under sonic-mgmt/tests folder, its parent
folder sonic-mgmt is automatically added to sys.path. That's why we can
use the new format to do importing.

Type of change

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

Approach

What is the motivation for this PR?

Since the sonic/tests folder is made as a package, adding subfolder of this package to sys.path would cause ambiguity issue of object identification. For example, in tests/common/platform/ssh_utils.py, the RunAnsibleModuleFail exception cannot be caught:

    try:
        ptfhost.shell('stat /root/.ssh/known_hosts')
    except RunAnsibleModuleFail:
        pass # files does not exist
    else:
        ptfhost.shell('ssh-keygen -f /root/.ssh/known_hosts -R ' + dut_ip)

The reason is that the raised exception has identification tests.common.errors.RunAnsibleModuleFail. However, the script here expects common.errors.RunAnsibleModuleFail here.

To avoid this issue, we should always explicitly use absolute import statement like 'from tests.common.xx import xx', or use relative import like 'from .common.xx import xx'. To prevent this issue, the code for adding tests/common folder to sys.path is also removed from tests/conftest.py.

How did you do it?

• Remove the code for adding tests/common to sys.path.
• Update all affected imports to either absolute import and relative import
• Replace the hacks in scripts under tests/platform_tests for adding subfolders to sys.path with relative import

How did you verify/test it?

Tested collect all the tests. No importing issue. Tested the scripts depend on ssh_util, the exception can be caught as expected.

Any platform specific information?

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

Documentation

[lgtm-com]

This pull request introduces 15 alerts and fixes 3 when merging a7863d4c33bec6ec70607dd1dc54638855f2f498 into 52b5b15 - view on LGTM.com

new alerts:

• 12 for Unused import
• 3 for Wrong number of arguments in a call

fixed alerts:

• 3 for Unused import

[lgtm-com]

This pull request introduces 6 alerts and fixes 3 when merging c4dd424a1a251bfa5952653c84b2b307dd0e7c70 into 77a6d1c - view on LGTM.com

new alerts:

• 3 for Unused import
• 3 for Wrong number of arguments in a call

fixed alerts:

• 3 for Unused import

[jleveque]

Retest this please

[tahmed-dev]

@wangxin can we instead of removing the current dir from sys path, catch the exception with its full name?

[tahmed-dev]

Good catch!

[wangxin]

@tahmed-dev Yes, we can specify full name to catch the exceptions. But it would be better to enforce importing from the tests package to avoid other potential similar issues that are kind of difficult to troubleshoot.

[lgtm-com]

This pull request introduces 3 alerts and fixes 3 when merging 225bc7a into 53222ad - view on LGTM.com

new alerts:

• 3 for Unused import

fixed alerts:

• 3 for Unused import

[lgtm-com]

This pull request introduces 3 alerts and fixes 3 when merging 4a13e38 into 53222ad - view on LGTM.com

new alerts:

• 3 for Unused import

fixed alerts:

• 3 for Unused import

[lolyu]

LGTM