Skip to content

Stop re-defining the argument spec in unit tests #11235

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Dec 1, 2025
Merged

Stop re-defining the argument spec in unit tests #11235

merged 2 commits into from
Dec 1, 2025

Conversation

@felixfontein
Copy link
Collaborator

@felixfontein felixfontein commented Nov 30, 2025

SUMMARY

DRY.

Ref: #11182 (comment)

ISSUE TYPE
  • Bugfix Pull Request
  • Test Pull Request
COMPONENT NAME

archive unit tests
java_keystore unit tests
nmlci unit tests

@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-11 Automatically create a backport for the stable-10 branch backport-12 Automatically create a backport for the stable-12 branch labels Nov 30, 2025
@ansibullbot
Copy link
Collaborator

ansibullbot commented Nov 30, 2025

cc @Mogztter @alcamie101 @bendoh @quidame
click here for bot help

@ansibullbot ansibullbot added bug This issue/PR relates to a bug module module plugins plugin (any type) tests tests unit tests/unit labels Nov 30, 2025
@ansibullbot

This comment was marked as outdated.

Sign in to view
@ansibullbot ansibullbot added ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Nov 30, 2025
@ansibullbot ansibullbot removed the ci_verified Push fixes to PR branch to re-run CI label Nov 30, 2025
@felixfontein felixfontein requested a review from russoz November 30, 2025 14:33
@ansibullbot ansibullbot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Nov 30, 2025
russoz
russoz approved these changes Dec 1, 2025
View reviewed changes
Copy link
Collaborator

@russoz russoz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hurraaaaaaayyyyyyyy!

That was a sneaky way of solving the issue, but it does the job. Thanks!!!

tests/unit/plugins/modules/test_nmcli.py
from ansible.module_utils.common.text.converters import to_text
from ansible_collections.community.general.plugins.modules import nmcli
from ansible.module_utils.basic import AnsibleModule
from ansible.module_utils.basic import AnsibleModule # noqa: F401 # pylint: disable=unused-import
Copy link
Collaborator

@russoz russoz Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this import being kept at all, since it is unused?

Copy link
Collaborator Author

@felixfontein felixfontein Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some mock patches further below that refer to names in here. From my experience they tend to not work when the code wasn't imported this way.

@felixfontein felixfontein merged commit fb2f34b into ansible-collections:main Dec 1, 2025
134 checks passed
@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Dec 1, 2025
@patchback
Copy link

patchback bot commented Dec 1, 2025 •
edited
Loading

Backport to stable-11: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply fb2f34b on top of patchback/backports/stable-11/fb2f34ba8593f5718c20ca663dd48f78f32df660/pr-11235

Backporting merged PR #11235 into main

  1. Ensure you have a local repo clone of your fork. Unless you cloned it
    from the upstream, this would be your origin remote.
  2. Make sure you have an upstream repo added as a remote too. In these
    instructions you'll refer to it by the name upstream. If you don't
    have it, here's how you can add it: 
    $ git remote add upstream https://github.com/ansible-collections/community.general.git
  3. Ensure you have the latest copy of upstream and prepare a branch
    that will hold the backported code: 
    $ git fetch upstream
$ git checkout -b patchback/backports/stable-11/fb2f34ba8593f5718c20ca663dd48f78f32df660/pr-11235 upstream/stable-11
  4. Now, cherry-pick PR Stop re-defining the argument spec in unit tests #11235 contents into that branch: 
    $ git cherry-pick -x fb2f34ba8593f5718c20ca663dd48f78f32df660
    If it'll yell at you with something like fatal: Commit fb2f34ba8593f5718c20ca663dd48f78f32df660 is a merge but no -m option was given., add -m 1 as follows instead: 
    $ git cherry-pick -m1 -x fb2f34ba8593f5718c20ca663dd48f78f32df660
  5. At this point, you'll probably encounter some merge conflicts. You must
    resolve them in to preserve the patch from PR Stop re-defining the argument spec in unit tests #11235 as close to the
    original as possible.
  6. Push this branch to your fork on GitHub: 
    $ git push origin patchback/backports/stable-11/fb2f34ba8593f5718c20ca663dd48f78f32df660/pr-11235
  7. Create a PR, ensure that the CI is green. If it's not — update it so that
    the tests and any other checks pass. This is it!
    Now relax and wait for the maintainers to process your pull request
    when they have some cycles to do reviews. Don't worry — they'll tell you if
    any improvements are necessary when the time comes!

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

@felixfontein felixfontein deleted the units branch December 1, 2025 05:59
@patchback
Copy link

patchback bot commented Dec 1, 2025 •
edited
Loading

Backport to stable-12: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-12/fb2f34ba8593f5718c20ca663dd48f78f32df660/pr-11235

Backported as #11239

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

@felixfontein
Copy link
Collaborator Author

felixfontein commented Dec 1, 2025

@russoz thanks for reviewing!

patchback bot pushed a commit that referenced this pull request Dec 1, 2025
@felixfontein @patchback
Stop re-defining the argument spec in unit tests (#11235)
87f28a2 
* Stop re-defining the argument spec in unit tests.

* Shut up linter.

(cherry picked from commit fb2f34b)
@patchback patchback bot mentioned this pull request Dec 1, 2025
Merged
felixfontein added a commit that referenced this pull request Dec 1, 2025
@patchback @felixfontein
[PR #11235/fb2f34ba backport][stable-12] Stop re-defining the argumen…
a2c7f9f 
…t spec in unit tests (#11239)

Stop re-defining the argument spec in unit tests (#11235)

* Stop re-defining the argument spec in unit tests.

* Shut up linter.

(cherry picked from commit fb2f34b)

Co-authored-by: Felix Fontein <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@russoz russoz russoz approved these changes

Assignees

No one assigned

Labels

backport-11 Automatically create a backport for the stable-10 branch backport-12 Automatically create a backport for the stable-12 branch bug This issue/PR relates to a bug module module plugins plugin (any type) tests tests unit tests/unit

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@felixfontein @ansibullbot @russoz