test: update regex for rust-coreutils stat -c output using double quotes

[blackboxsw]

Proposed Commit Message

test: update regex for rust-coreutils stat -c output using double quotes

Will be fixed in upstream rust-coreutils per
https://github.com/uutils/coreutils/issues/8789. But,
this may take a bit to SRU to Ubuntu Questing.

Additional Context

Unhappy Jenkins job representing this failure only on Questing
https://jenkins.canonical.com/server-team/view/cloud-init/job/cloud-init-integration-questing-ec2-generic/lastSuccessfulBuild/testReport/junit/tests.integration_tests.modules.test_wireguard/TestWireguard/test_wireguard_stat__c___N___etc_wireguard_wg0_conf___etc_wireguard_wg0_conf__/

Test Steps

# single quote test case
CLOUD_INIT_PLATFORM=lxd_vm CLOUD_INIT_OS_IMAGE=plucky  tox -e integration-tests -- tests/integration_tests/modules/test_wireguard::TestWireguard::test_wireguard

# double quote test case

CLOUD_INIT_PLATFORM=lxd_vm CLOUD_INIT_OS_IMAGE=questing tox -e integration-tests -- tests/integration_tests/modules/test_wireguard::TestWireguard::test_wireguard

Merge type

• Squash merge using "Proposed Commit Message"
• Rebase and merge unique commits. Requires commit messages per-commit each referencing the pull request number (#<PR_NUM>)

[holmanb]

@blackboxsw Nice work! Thanks for identifying the cause and reporting it upstream. Is there a bug against questing for this too?

However, this test fix doesn't look correct. The comparison uses in, so I wouldn't expect regex to work.

As you mentioned, the cause of this failure is known and based on the pace of development in uutils I expect this to be fixed soon in upstream. Rather than complicating the test logic with regex, perhaps we can just wait for the fix to be SRU'd?

[holmanb]

.

[blackboxsw]

@blackboxsw Nice work! Thanks for identifying the cause and reporting it upstream. Is there a bug against questing for this too?

However, this test fix doesn't look correct. The comparison uses in, so I wouldn't expect regex to work.

As you mentioned, the cause of this failure is known and based on the pace of development in uutils I expect this to be fixed soon in upstream. Rather than complicating the test logic with regex, perhaps we can just wait for the fix to be SRU'd?

I've just filed a Bug against rust-coretutils in Ubuntu Questing LP: #2126954 referencing this upstream issue and ongoing work. It looks like the fix author is planning on test coverage upstream, we can then see what sort of timeline upstream fix is, then we have SRU queue to await as well if release team deems this priority enough for SRU. I'd really like to get our integration tests to green and this current "bug" is preventing use from getting good signal on any test runs without manual review. Why don't we give it until Friday to sort whether SRU is planned to fix this. If not, I'd like to get to a 'clean sheet' for integration test runs.

[blackboxsw]

Given different answers from release team. This may take a while to trickle into Ubuntu Questing. For the time being let us fixup integration test failures for this delta. We can back this out if LP: #2126954 is resolved against questing.

[holmanb]

@blackboxsw Do the other tests still pass?

It isn't generally safe to just switch substring matches to regex matches - even if it works sometimes. I seem to recall that this kind of change has caused tests to fail multiple times in the past (which is why I initially proposed waiting). I see some regex metacharacters in the strings that were previously substring matches, so I'm suspicious that this will cause failures.

[blackboxsw]

@blackboxsw Do the other tests still pass?

I had run this on plucky and questing to confirm differences, but I've just kicked off noble and jammy with successs as well

CLOUD_INIT_PLATFORM=lxd_vm CLOUD_INIT_OS_IMAGE=jammy .tox/integration-tests/bin/pytest tests/integration_tests/modules/test_wireguard.py::TestWireguard::test_wireguard
...
======================== 9 passed in 143.24s (0:02:23) =========================
CLOUD_INIT_PLATFORM=lxd_vm CLOUD_INIT_OS_IMAGE=noble .tox/integration-tests/bin/pytest tests/integration_tests/modules/test_wireguard.py::TestWireguard::test_wireguard
...
======================== 9 passed in 142.40s (0:02:22) =========================

. I seem to recall that this kind of change has caused tests to fail multiple times in the past (which is why I initially proposed waiting). I see some regex metacharacters in the strings that were previously substring matches, so I'm suspicious that this will cause failures.

It isn't generally safe to just switch substring matches to regex matches - even if it works sometimes....

While I understand the merit of this suggestion, and also recall the multiple cases we've hit in the past, this isn't switching to regex strict matches but re.search which will still match a substring. since the substring matching (and related regex is a one character difference I think we aren't exposed here. We're matching a quoted path not stuff with a variable amount of whitespace or mulitple words etc.

[holmanb]

@blackboxsw I got confused reading my own words in the unquoted reply, but I think I was able to parse what I see.

It isn't generally safe to just switch substring matches to regex matches - even if it works sometimes....

While I understand the merit of this suggestion, and also recall the multiple cases we've hit in the past, this isn't switching to regex strict matches but re.search which will still match a substring. since the substring matching (and related regex is a one character difference I think we aren't exposed here. We're matching a quoted path not stuff with a variable amount of whitespace or mulitple words etc.

After taking another look at this, in this specific case I think this change is fine. Previously I saw some of the regex special characters in the first argument (where both arguments are on the same line) and thought that it would cause a much broader match than we wanted.

When modifying parametrized tests it would be nice if we could avoid having to modify the behavior of (and therefore have to verify) other unaffected test cases. For example, rather than:

-       assert expected_out in result.stdout
+       assert re.search(expected_out, result.stdout)

Something like this would be easier to verify for correctness by only changing the affected test:

+       if isinstance(result.stdout, re.Pattern):
+           assert re.search(expected_out, result.stdout)
+           return
        assert expected_out in result.stdout

where the changed parametrized test would then look like:

re.compile(r"['\"]/etc/wireguard/wg0.conf['\"]")

Since we've both already gone through and verified the strings (and tests pass), I'm fine with moving forward with this PR as is. Thanks for the fix @blackboxsw!