Skip to content
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

[Bugfix] Fix import error in device_utils.py. #16860

Merged

Conversation

yutongzhang-microsoft
Copy link
Contributor

@yutongzhang-microsoft yutongzhang-microsoft commented Feb 8, 2025

Description of PR

In PR #15605, the platform_api_conn fixture was relocated to a common location. The fixture start_platform_api_service used by it is not imported in this script, which will casue error fixture 'start_platform_api_service' not found. This PR addresses the issue by moving fixture start_platform_api_service to a common place tests/conftest.py.

Summary:
Fixes # (issue)

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • New Test case
    • Skipped for non-supported platforms
  • Test case improvement

Back port request

  • 202012
  • 202205
  • 202305
  • 202311
  • 202405
  • 202411

Approach

What is the motivation for this PR?

In PR #15605, the platform_api_conn fixture was relocated to a common location. The fixture start_platform_api_service used by it is not imported in this script, which will casue error fixture 'start_platform_api_service' not found. This PR addresses the issue by moving fixture start_platform_api_service to a common place tests/conftest.py.

How did you do it?

This PR addresses the issue by moving fixture start_platform_api_service to a common place tests/conftest.py.

How did you verify/test it?

Test locally to run the test script test_reload_dpu.py. There is no such import error.

Any platform specific information?

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

Documentation

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@@ -18,6 +18,7 @@
from tests.common.mellanox_data import is_mellanox_device
from tests.common.platform.reboot_timing_constants import SERVICE_PATTERNS, OTHER_PATTERNS, SAIREDIS_PATTERNS, \
OFFSET_ITEMS, TIME_SPAN_ITEMS, REQUIRED_PATTERNS
from tests.platform_tests.api.conftest import start_platform_api_service # noqa F401
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we get rid of this flake error noqa? It is flowing down in all the code which does not make code clean.

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vvolam
Copy link
Contributor

vvolam commented Feb 10, 2025

@yutongzhang-microsoft Please backport the fix to 202411 branch as well, where #15605 changes are present.

Copy link
Contributor

@vvolam vvolam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Thank you!

@wangxin wangxin merged commit ba7e27b into sonic-net:master Feb 10, 2025
14 checks passed
@yutongzhang-microsoft yutongzhang-microsoft deleted the yutongzhang/fix_import_error branch February 10, 2025 08:13
mssonicbld pushed a commit to mssonicbld/sonic-mgmt that referenced this pull request Mar 4, 2025
What is the motivation for this PR?
In PR sonic-net#15605, the platform_api_conn fixture was relocated to a common location. The fixture start_platform_api_service used by it is not imported in this script, which will casue error fixture 'start_platform_api_service' not found. This PR addresses the issue by moving fixture start_platform_api_service to a common place tests/conftest.py.

How did you do it?
This PR addresses the issue by moving fixture start_platform_api_service to a common place tests/conftest.py.

How did you verify/test it?
Test locally to run the test script test_reload_dpu.py. There is no such import error.
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202411: #17339

mssonicbld pushed a commit that referenced this pull request Mar 5, 2025
What is the motivation for this PR?
In PR #15605, the platform_api_conn fixture was relocated to a common location. The fixture start_platform_api_service used by it is not imported in this script, which will casue error fixture 'start_platform_api_service' not found. This PR addresses the issue by moving fixture start_platform_api_service to a common place tests/conftest.py.

How did you do it?
This PR addresses the issue by moving fixture start_platform_api_service to a common place tests/conftest.py.

How did you verify/test it?
Test locally to run the test script test_reload_dpu.py. There is no such import error.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants