-
Notifications
You must be signed in to change notification settings - Fork 49
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
Fix error in windows network plugins #955
base: main
Are you sure you want to change the base?
Conversation
Add optional params to _get_config_value to split value according to a list of character Regression added in fox-it#870
Add optional params to _get_config_value to split value according to a list of character Regression added in fox-it#870
Add optional params to _get_config_value to split value according to a list of character Regression added in fox-it#870
Add optional params to _get_config_value to split value according to a list of character Regression added in fox-it#870
Add optional params to _get_config_value to split value according to a list of character Regression added in fox-it#870
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.
I can confirm this fixes the issue introduced by our changes in #870.
Could you apply the following patch to the test file diff --git a/tests/plugins/os/windows/test_network.py b/tests/plugins/os/windows/test_network.py
index a9237fc..2eac5bd 100644
--- a/tests/plugins/os/windows/test_network.py
+++ b/tests/plugins/os/windows/test_network.py
@@ -257,7 +257,7 @@ def test_windows_network_none(
"DhcpIPAddress": "192.168.0.10",
"IPAddress": "10.0.0.10",
"DhcpNameServer": "192.168.0.2",
- "NameServer": "10.0.0.2",
+ "NameServer": "10.0.0.2 10.0.0.3",
"SubnetMask": "255.255.255.0",
"DhcpSubnetMask": "255.255.255.0",
"VlanID": 10,
@@ -285,7 +285,7 @@ def test_windows_network_none(
},
{
"ip": ["10.0.0.10"],
- "dns": ["10.0.0.2"],
+ "dns": ["10.0.0.3", "10.0.0.2"],
"gateway": ["10.0.0.1"],
"mac": ["FE:EE:EE:EE:EE:ED"],
"subnetmask": ["255.255.255.0"], |
Coding style : Optional -> | None Co-authored-by: Computer Network Investigation <[email protected]>
Patch has been applied |
* rename params to sep * Change separator value to str from list of str Co-authored-by: Erik Schamper <[email protected]>
Changes applied |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #955 +/- ##
==========================================
- Coverage 77.90% 75.99% -1.92%
==========================================
Files 324 324
Lines 27857 27861 +4
==========================================
- Hits 21702 21172 -530
- Misses 6155 6689 +534
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Can you look at why the unit tests are failing?
Seems to be a race condition. The following should fix it: diff --git a/tests/plugins/os/windows/test_network.py b/tests/plugins/os/windows/test_network.py
index a9237fc..5a7898d 100644
--- a/tests/plugins/os/windows/test_network.py
+++ b/tests/plugins/os/windows/test_network.py
@@ -285,7 +285,7 @@ def test_windows_network_none(
},
{
"ip": ["10.0.0.10"],
- "dns": ["10.0.0.3", "10.0.0.2"],
+ "dns": ["10.0.0.2", "10.0.0.3"],
"gateway": ["10.0.0.1"],
"mac": ["FE:EE:EE:EE:EE:ED"],
"subnetmask": ["255.255.255.0"],
@@ -346,8 +346,8 @@ def test_network_dhcp_and_static(
gateways.update(interface.gateway)
macs.update(interface.mac)
- assert interface.ip == expected["ip"]
- assert interface.dns == expected["dns"]
+ assert sorted(map(str, interface.ip)) == expected["ip"]
+ assert sorted(map(str, interface.dns)) == expected["dns"]
assert interface.gateway == expected["gateway"]
assert interface.mac == expected["mac"]
assert interface.network == expected["network"] |
…lated to list order)
Done |
Fix issue #951