-
Notifications
You must be signed in to change notification settings - Fork 1k
ci: feature values should be read from client under test #6738
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?
ci: feature values should be read from client under test #6738
Conversation
0329ec7 to
3d56f96
Compare
holmanb
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.
One question, besides the failing CI.
|
|
||
| from cloudinit import features, lifecycle | ||
| from cloudinit.subp import subp | ||
| from cloudinit.util import is_true |
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.
Why is this being imported and used?
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 reason to use is_true, is because the value coming back from get_feature_flag_value will either be a string 'False' or 'True', we can wrap that in is_true() to align with the expected return value from network_wait_loggged() which parses the full log to ensure the expected 'wait' message was included. Alternatively, we could either:
- explicitly check for the return value of 'True' from get_feature_flag and then use that to be our assertion for the return of network_wait_logged(...).
Would you prefer one of these approaches instead?
- convert True/False strings to python boolean?
--- a/tests/integration_tests/datasources/test_nocloud.py
+++ b/tests/integration_tests/datasources/test_nocloud.py
@@ -7,7 +7,6 @@ from pycloudlib.lxd.instance import LXDInstance
from cloudinit import features, lifecycle
from cloudinit.subp import subp
-from cloudinit.util import is_true
from tests.integration_tests.instances import IntegrationInstance
from tests.integration_tests.integration_settings import PLATFORM
from tests.integration_tests.releases import CURRENT_RELEASE, FOCAL
@@ -131,7 +130,7 @@ def test_nocloud_seedfrom_vendordata(client: IntegrationInstance):
assert "seeded_vendordata_test_file" in client.execute("ls /var/tmp")
assert network_wait_logged(
client.execute("cat /var/log/cloud-init.log")
- ) == is_true(get_feature_flag_value(client, "MANUAL_NETWORK_WAIT"))
+ ) is get_feature_flag_value(client, "MANUAL_NETWORK_WAIT")
SMBIOS_USERDATA = """\
diff --git a/tests/integration_tests/util.py b/tests/integration_tests/util.py
index 6e86b6775..8ccbb85cd 100644
--- a/tests/integration_tests/util.py
+++ b/tests/integration_tests/util.py
@@ -603,6 +603,8 @@ def get_feature_flag_value(client: "IntegrationInstance", key):
).strip()
if "NameError" in value:
raise NameError(f"name '{key}' is not defined")
+ if value in ("True", "False"):
+ return value == "True"
return value
- Perform the string check above call-site before our assertion
diff --git a/tests/integration_tests/datasources/test_nocloud.py b/tests/integration_tests/datasources/test_nocloud.py
index 0bfe29668..100cd9c93 100644
--- a/tests/integration_tests/datasources/test_nocloud.py
+++ b/tests/integration_tests/datasources/test_nocloud.py
@@ -7,7 +7,6 @@ from pycloudlib.lxd.instance import LXDInstance
from cloudinit import features, lifecycle
from cloudinit.subp import subp
-from cloudinit.util import is_true
from tests.integration_tests.instances import IntegrationInstance
from tests.integration_tests.integration_settings import PLATFORM
from tests.integration_tests.releases import CURRENT_RELEASE, FOCAL
@@ -129,9 +128,11 @@ def test_nocloud_seedfrom_vendordata(client: IntegrationInstance):
client.restart()
assert client.execute("cloud-init status").ok
assert "seeded_vendordata_test_file" in client.execute("ls /var/tmp")
- assert network_wait_logged(
- client.execute("cat /var/log/cloud-init.log")
- ) == is_true(get_feature_flag_value(client, "MANUAL_NETWORK_WAIT"))
+ wait_log = "True" == get_feature_flag_value(client, "MANUAL_NETWORK_WAIT")
+ assert (
+ network_wait_logged(client.execute("cat /var/log/cloud-init.log"))
+ == wait_log
+ )
acfb107 to
0477029
Compare
Fix 2 Jammy integration test run failures which are due to not sourcing the features settings that are installed on the system under test.
Integration tests should use
get_feature_flag_valuefrom the client instead of looking at the local repo's features.<FEATURE_NAME> value.Proposed Commit Message
ci: feature values should be read from client under testAdditional Context
Github action faliure for jammy
Test Steps
Merge type