fix(tests): make user_groups integration tests distro-aware by mmummigatti · Pull Request #6889 · canonical/cloud-init

mmummigatti · 2026-05-20T08:53:30Z

The integration tests for users_groups module had Ubuntu-specific
assumptions that prevented them from running on RHEL-family systems.

Changes:

Add skipif decorators to Ubuntu-specific warning tests (test_initial_warnings,
test_nopassword_unlock_warnings) since warning formats differ across distros
Remove redundant IS_UBUNTU checks inside skipif-decorated tests
Restore test_sudoers_includedir decorators with distro-aware skipif logic:
'IS_UBUNTU and CURRENT_RELEASE < JAMMY' allows the test to run on RHEL
and modern Ubuntu while skipping only on old Ubuntu releases lacking
required sudo version
Add JAMMY import needed for the distro-aware skipif condition

These changes ensure tests either:

Run on all distros with appropriate handling, or
Skip gracefully on distros where the test isn't applicable

This allows the test suite to pass on both Ubuntu and RHEL-family systems.

ani-sinha

Can you please check that tests pass with both RHEL and Ubuntu images?

ani-sinha · 2026-05-20T09:53:25Z

+            if IS_UBUNTU and CURRENT_RELEASE > NOBLE
            else []
        )
+


whitespace.

Please address this comment.

These blank lines were added to satisfy tox formatting requirements.
I've updated the commit message to document this.

ani-sinha · 2026-05-20T09:54:38Z

        class_client.execute("passwd -d foobar")
        class_client.instance.clean()
        class_client.restart()
+


whitespace.

Please address.

These blank lines were added to satisfy tox formatting requirements.
I've updated the commit message to document this.

ani-sinha · 2026-05-29T12:36:50Z

    IS_UBUNTU,
    JAMMY,
    NOBLE,
+


Is this space required?

If this is for tox formatting, please add a comment to that effect in the commit log.

ani-sinha · 2026-05-29T12:44:53Z

+
        verify_clean_boot(
            class_client,
-            ignore_warnings=True,  # ignore warnings about existing groups


why remove this comment? Its not related to the change.

ani-sinha · 2026-05-29T12:45:25Z

            require_warnings=warnings,
-        )
-
+       )


I think this is simply whitespace issues. There should be no changes here. Can you double check?

The integration tests for users_groups module had Ubuntu-specific assumptions that prevented them from running on RHEL-family systems. Changes: - Add skipif decorators to Ubuntu-specific warning tests - Remove redundant IS_UBUNTU checks inside skipif-decorated tests - Restore test_sudoers_includedir decorators with distro-aware skipif logic: 'IS_UBUNTU and CURRENT_RELEASE < JAMMY' allows test to run on RHEL and modern Ubuntu while skipping old Ubuntu releases - Add JAMMY import needed for the distro-aware skipif condition

ani-sinha

LGTM.

holmanb

The concept is good, but this isn't ready for review. Please don't introduce arbitrary whitespace changes in PRs.

holmanb · 2026-06-12T19:04:12Z

+            if IS_UBUNTU and CURRENT_RELEASE > NOBLE
            else []
        )
+


Please address this comment.

holmanb · 2026-06-12T19:04:34Z

        class_client.execute("passwd -d foobar")
        class_client.instance.clean()
        class_client.restart()
+


Please address.

ani-sinha · 2026-06-15T02:54:55Z

@holmanb Medha told me offline that those new lines were required to satisfy tox formatting. @mmummigatti if my understanding is correct, please add a comment to that effect in the commit log.

holmanb · 2026-06-15T14:56:56Z

Github Actions runs format checks on every PR push. This justification doesn't make sense. Why would a PR that changes this file suddenly require other whitespace changes in just this file?

ani-sinha · 2026-06-15T23:17:20Z

If you do not want to have the whitespace changes, please say so. Running tox -e do_format is a standard procedure for other PRs I have submitted. Tox only checks for formatting changes in files we change. This has happend to me also in the past.

@mmummigatti please revert the whitespace changes and also remove the comment in the description.

mmummigatti · 2026-06-16T06:27:39Z

Reverted whitespace-only changes.

ani-sinha · 2026-06-16T07:35:39Z

I do not understand why these are part of the diff. There should be no changes on these lines if we disregard tox formating issues. Please double check.

Fixed - removed the unnecessary whitespace changes. The current diff now only contains the functional skipif decorators.

Did you forget to force push? I do not see any chnages.

ani-sinha · 2026-06-16T08:00:23Z

https://docs.cloud-init.io/en/23.4/development/#pull-request-checklist

Does this instruction not apply to functional tests? I do not see gitlab actions ran any pylint/tox checks here. So its very confusing for contributors.

holmanb · 2026-06-16T14:56:46Z

What are you trying to say?

I don't believe that this claim is accurate.

Please provide a reproducer. I have not experienced that and couldn't reproduce it here.

You didn't see it because Github Actions runs are gated on maintainer approval for first-time contributors.

ani-sinha · 2026-06-16T15:17:03Z

The tone of your comment impled medha left those whitespaces accidentally and did not really intentionally add them to satisfy tox. I found that quite frankly hurtful. Is there any reason to believe she was lying?

holmanb · 2026-06-16T18:14:02Z

Let's keep this on topic, please.

I have never seen tox -e do_format behave the way that you describe, so I need to understand what causes it so that the tooling or contribution docs can be fixed. After re-reading[1] and testing the code, I still see no evidence that tox only checks the files we change. I similarly don't see any version of the formatters that would cause the whitespace changes here. What am I missing?

[1] tox -e do_format just runs some formatters - it has no awareness of git (the same is true of the relevant github workflow). Similarly the versions of the formatters are fixed.

ani-sinha · 2026-06-24T13:55:12Z

Is there anything left to do here?

holmanb · 2026-06-29T21:42:35Z

@mmummigatti Thank you for dropping the whitespace changes. I think that the CI test failures have been fixed upstream. Please merge the latest version of the main branch. I think that should help get a passing CI run.

@mmummigatti @ani-sinha If you happen to figure out what caused the unexpected whitespace changes, I would still appreciate understanding the root cause. Otherwise I will assume that the issue has resolved itself.

ani-sinha suggested changes May 20, 2026

View reviewed changes

ani-sinha reviewed May 26, 2026

View reviewed changes

Comment thread tests/integration_tests/modules/test_users_groups.py

mmummigatti force-pushed the rhel-fix-user-group branch 2 times, most recently from 831efb8 to cc3f091 Compare May 29, 2026 09:00

ani-sinha suggested changes May 29, 2026

View reviewed changes

Comment thread tests/integration_tests/modules/test_users_groups.py

mmummigatti force-pushed the rhel-fix-user-group branch from cc3f091 to c2f7817 Compare May 29, 2026 10:23

ani-sinha suggested changes May 29, 2026

View reviewed changes

mmummigatti force-pushed the rhel-fix-user-group branch from c2f7817 to 40758ef Compare June 9, 2026 08:54

ani-sinha approved these changes Jun 10, 2026

View reviewed changes

holmanb requested changes Jun 12, 2026

View reviewed changes

mmummigatti force-pushed the rhel-fix-user-group branch from 40758ef to f171b60 Compare June 15, 2026 05:26

mmummigatti force-pushed the rhel-fix-user-group branch from f171b60 to fd0ee00 Compare June 16, 2026 06:18

ani-sinha suggested changes Jun 16, 2026

View reviewed changes

mmummigatti force-pushed the rhel-fix-user-group branch 2 times, most recently from 2d4142a to 6503a0e Compare June 17, 2026 11:19

ani-sinha approved these changes Jun 17, 2026

View reviewed changes

Uh oh!

Conversation

mmummigatti commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ani-sinha left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ani-sinha left a comment

Choose a reason for hiding this comment

Uh oh!

holmanb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ani-sinha commented Jun 15, 2026

Uh oh!

holmanb commented Jun 15, 2026

Uh oh!

ani-sinha commented Jun 15, 2026

Uh oh!

mmummigatti commented Jun 16, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ani-sinha commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

holmanb commented Jun 16, 2026

Uh oh!

ani-sinha commented Jun 16, 2026

Uh oh!

holmanb commented Jun 16, 2026

Uh oh!

ani-sinha commented Jun 24, 2026

Uh oh!

holmanb commented Jun 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mmummigatti commented May 20, 2026 •

edited

Loading

ani-sinha commented Jun 16, 2026 •

edited

Loading