-
Notifications
You must be signed in to change notification settings - Fork 77
Test: Support for PFs in tests fxitures #1274
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
base: main
Are you sure you want to change the base?
Conversation
DawidWesierski4
left a comment
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.
This is generally not ready for the review i think
please remove all comments add clear git comment, please a clear
How to tests: would be a lifre sever
This also needs to be rebased and squashed as the 40 comments are not really helping :(
dd7e1e1 to
a25863b
Compare
9969ac9 to
f3f59a9
Compare
f3f59a9 to
e116236
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.
Pull Request Overview
This PR refactors test fixtures to support both Virtual Functions (VFs) and Physical Functions (PFs) as test interfaces. A new test configuration parameter interface_type enables tests to dynamically choose between VF and PF configurations. The refactoring focuses on single-node tests, with dual-node tests planned for a subsequent PR.
- Introduces
InterfaceSetupclass to manage interface creation and cleanup - Replaces hardcoded
host.vfsreferences with dynamic interface selection - Adds
setup_interfacesfixture for function-scoped interface management
Reviewed Changes
Copilot reviewed 62 out of 62 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/validation/common/nicctl.py | Adds InterfaceSetup class with methods for creating VFs/PFs and managing cleanup |
| tests/validation/conftest.py | Adds setup_interfaces fixture and imports InterfaceSetup |
| tests/validation/configs/examples/test_config.yaml | Documents new interface_type configuration parameter |
| tests/validation/tests/single//test_.py | Replaces nic_port_list fixture with setup_interfaces and adds interface type selection logic |
| tests/validation/tests/single/xdp//test_.py | Removes unused test_config parameter |
| tests/validation/tests/single/kernel_socket//test_.py | Removes unused test_config parameter and adds explanatory comments for hardcoded interfaces |
| tests/validation/mtl_engine/ffmpeg_app.py | Adds nic_port_list parameter to execute_test and execute_test_rgb24 functions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| """ | ||
| Creates VFs and binds them into dpdk or bind PF into dpdk. | ||
| :param interface_type: VF - create X VFs on firt available test adapter, |
Copilot
AI
Nov 17, 2025
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.
Corrected spelling of 'firt' to 'first'.
| :param interface_type: VF - create X VFs on firt available test adapter, | |
| :param interface_type: VF - create X VFs on first available test adapter, |
| user: user | ||
| password: password | ||
| proxy: false | ||
| interface_type: VF - create X VFs on firt available test adapter, |
Copilot
AI
Nov 17, 2025
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.
Corrected spelling of 'firt' to 'first'.
| interface_type: VF - create X VFs on firt available test adapter, | |
| interface_type: VF - create X VFs on first available test adapter, |
| case_id = case_id[: case_id.rfind("(") - 1] if "(" in case_id else case_id | ||
|
|
||
| nic_port_list = host.vfs | ||
| video_size, fps = decode_video_format_16_9(video_format) |
Copilot
AI
Nov 17, 2025
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.
The removal of init_test_logging() call breaks the logging functionality. The _log_timestamp global variable used elsewhere in the module will not be initialized, causing potential issues. The call should be restored at the beginning of the function.
| for host in hosts: | ||
| if getattr(host.topology.extra_info, "custom_interface", None): | ||
| selected_interfaces[host.name] = [ | ||
| host.topology.extra_info["custom_interface"] |
Copilot
AI
Nov 17, 2025
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.
Inconsistent attribute access: line 112 uses getattr with a default, but line 114 uses dictionary-style access which could raise KeyError if the attribute exists but isn't a dict. Use consistent access method: getattr(host.topology.extra_info, 'custom_interface', None)
| host.topology.extra_info["custom_interface"] | |
| getattr(host.topology.extra_info, "custom_interface", None) |
Refactor fixtures to support either VFs and PFs in tests as test interfaces.
Use new test config param for choosing if tests shall use VFs or PFs: "interface_type"
Single node tests refactored except kernel_socket and xdp (different network interfaces - needs another approach)
Dual node tests will be done in next PR