test: ensure role gathers the facts it uses by having test clear_facts before include_role

[richm]

The role gathers the facts it uses. For example, if the user uses
ANSIBLE_GATHERING=explicit, the role uses the setup module with the
facts and subsets it requires.

This change allows us to test this. Before every role invocation, the test
will use meta: clear_facts so that the role starts with no facts.

Create a task file tests/tasks/run_role_with_clear_facts.yml to do the tasks
to clear the facts and run the role. Note that this means we don't need to
use gather_facts for the tests.

Some vars defined using ansible_facts have been changed to be defined with
set_fact instead. This is because of the fact that vars are lazily
evaluated - the var might be referenced when the facts have been cleared, and
will issue an error like ansible_facts["distribution"] is undefined. This is
typically done for blocks that have a when condition that uses ansible_facts
and the block has a role invocation using run_role_with_clear_facts.yml
These have been rewritten to define the when condition using set_fact. This
is because the when condition is evaluated every time a task is invoked in the
block, and if the facts are cleared, this will raise an undefined variable error.

Signed-off-by: Rich Megginson rmeggins@redhat.com

Summary by Sourcery

Ensure the timesync role is always executed with a clean fact set in tests by routing test role invocations through a helper task file that clears facts before including the role.

New Features:

• Add a reusable test task file that clears Ansible facts and then runs the linux-system-roles.timesync role, with optional control over tasks_from, public vars export, and failure handling.

Enhancements:

• Refine test setup to derive commonly used interface data from a dedicated fact variable instead of directly from ansible_facts.

Tests:

• Update timesync-related test playbooks to use the new helper task for role execution, remove explicit gather_facts usage where no longer needed, and adjust tests to rely on the new fact-backed default interface variable.

[sourcery-ai]

Reviewer's Guide

Introduces a shared test helper task file that clears Ansible facts before invoking the linux-system-roles.timesync role, refactors tests to use it instead of direct include_role/roles and gather_facts, and adjusts tests to rely on a set_fact-derived interface variable so they remain robust when facts are cleared.

File-Level Changes

Change	Details	Files
Add a reusable task helper that clears facts then runs the timesync role, supporting optional public vars and failure-handling semantics.	Create tests/tasks/run_role_with_clear_facts.yml that first runs meta: clear_facts then includes linux-system-roles.timesync with configurable tasks_from and public options. Implement conditional path for __sr_failed_when == false that wraps the role include in a block/rescue and swallows failures via a debug message. Provide a default path that runs the role normally when __sr_failed_when is undefined or true.	tests/tasks/run_role_with_clear_facts.yml
Refactor tests to use the new run_role_with_clear_facts helper instead of direct role inclusion and remove now-unneeded gather_facts/roles declarations.	Replace include_role: linux-system-roles.timesync invocations with include_tasks: tasks/run_role_with_clear_facts.yml, wiring __sr_public and other role vars through vars blocks. Remove play-level gather_facts: true/false and roles: linux-system-roles.timesync where the new helper is used, moving role execution into explicit tasks. Update cleanup and provider-related tests to call the role via the helper while preserving their existing vars and assertions.	tests/tests_options.yml tests/tests_ntp_ptp.yml tests/tests_ntp_provider6.yml tests/tests_chrony.yml tests/tests_default.yml tests/tests_default_vars.yml tests/tests_ntp.yml tests/tests_ntp_provider1.yml tests/tests_ntp_provider2.yml tests/tests_ntp_provider3.yml tests/tests_ntp_provider4.yml tests/tests_ntp_provider5.yml tests/tasks/cleanup.yml
Ensure tests that rely on the default IPv4 interface remain valid when facts are cleared by deriving an intermediate fact variable in setup tasks and using it instead of ansible_facts lookups.	In tests/tasks/setup.yml, introduce a set_fact task that stores ansible_facts['default_ipv4']['interface'] into __timesync_default_ipv4_interface so it survives clear_facts usage patterns within blocks. Update PTP- and NTP-related tests to reference __timesync_default_ipv4_interface in vars and commands instead of directly accessing ansible_facts['default_ipv4']['interface'].	tests/tasks/setup.yml tests/tests_ptp_multi.yml tests/tests_ptp_single.yml tests/tests_ntp_ptp.yml

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[richm]

[citest]

[sourcery-ai]

Hey - I've left some high level feedback:

• The new __timesync_default_ipv4_interface is set via set_fact in tasks/setup.yml, but meta: clear_facts in run_role_with_clear_facts.yml will clear facts set by set_fact as well, so anything relying on that value after the role run (or inside the role via timesync_ptp_domains) may see it undefined; consider storing this in a normal variable (not via set_fact) or moving the clear_facts so it does not wipe this value.
• Several plays now depend on __timesync_default_ipv4_interface in play-level vars (e.g. timesync_ptp_domains), which are evaluated lazily; it would be safer to ensure this variable is defined before any task that may reference those vars, or to compute the interface directly in the vars to avoid ordering issues between tasks/setup.yml and the role include.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The new `__timesync_default_ipv4_interface` is set via `set_fact` in `tasks/setup.yml`, but `meta: clear_facts` in `run_role_with_clear_facts.yml` will clear facts set by `set_fact` as well, so anything relying on that value after the role run (or inside the role via `timesync_ptp_domains`) may see it undefined; consider storing this in a normal variable (not via `set_fact`) or moving the `clear_facts` so it does not wipe this value.
- Several plays now depend on `__timesync_default_ipv4_interface` in play-level vars (e.g. `timesync_ptp_domains`), which are evaluated lazily; it would be safer to ensure this variable is defined before any task that may reference those vars, or to compute the interface directly in the vars to avoid ordering issues between `tasks/setup.yml` and the role include.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[richm]

[citest]

[richm]

[citest]

[richm]

[citest]

[richm]

[citest]

[richm]

[citest]