Skip to content
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

Add Jinja2 tests for testing types #37517

Closed
wants to merge 1 commit into from

Conversation

dagwieers
Copy link
Contributor

@dagwieers dagwieers commented Mar 16, 2018

SUMMARY

So Jinja2 only has tests for testing string types, or iterable types,
which is quite limiting. If you need to test a boolean value in Jinja2
the only option is to test it is not a string and convert it to a string
and compare it to 'True' or 'False'.

So these tests are essential constructs in playbooks and templates.

This includes tests for:

  • boolean
  • true
  • false
  • list
  • integer
  • float

So you can simply do things like:

{% if result.value is false %}

or

{% if result.value is list %}

Just like you already can do

{% if result.value is none %}

and

{% if result.value is mapping %}

See the included integration tests for a complete explanation of what Jinja2 already can do, and how this is insufficient in some cases where this would be useful.

This is related to #37514

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

Jinja2 core tests

ANSIBLE VERSION

v2.6

@dagwieers dagwieers requested a review from abadger March 16, 2018 13:44
@ansibot ansibot added feature This issue/PR relates to a feature request. needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Mar 16, 2018
@dagwieers dagwieers force-pushed the jinja2-type-tests branch 8 times, most recently from 32c5efd to df36426 Compare March 16, 2018 14:45
@mkrizek mkrizek removed the needs_triage Needs a first human triage before being processed. label Mar 16, 2018
@ansibot ansibot added support:community This issue/PR relates to code supported by the Ansible community. test This PR relates to tests. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Mar 16, 2018
@ansible ansible deleted a comment from ansibot Mar 16, 2018
@dagwieers dagwieers force-pushed the jinja2-type-tests branch 4 times, most recently from 459373b to 9fc9cd1 Compare March 16, 2018 17:31
@ansible ansible deleted a comment from ansibot Mar 16, 2018
@ansibot ansibot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Mar 16, 2018
@ansible ansible deleted a comment from ansibot Mar 16, 2018
@ansibot
Copy link
Contributor

ansibot commented Mar 16, 2018

@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Mar 16, 2018
@dagwieers dagwieers force-pushed the jinja2-type-tests branch 2 times, most recently from 27375cc to 571fa20 Compare March 17, 2018 00:47
@ansibot ansibot added the affects_2.6 This issue/PR affects Ansible v2.6 label May 18, 2018
felixfontein added a commit to felixfontein/ansible that referenced this pull request Jun 1, 2018


# NOTE: The existing Jinja2 'sequence' test matches strings and dictionaries
def test_list(value):
Copy link
Member

Choose a reason for hiding this comment

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

Now that #39924 has been merged into devel you can:

from functools import partial

from ansible.module_utils.common.collections import is_sequence

test_list = partial(is_sequence, include_strings=False)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not impressed, sorry. I don't think this will make it easier to upstream. I don't need an Ansible-only solution.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not talking about upstream. This is about this PR, which currently doesn't need to have duplicate code, like one in devel.

acozine pushed a commit that referenced this pull request Jun 28, 2018
)

* Also marking non-string defaults.

* Adding list filter from #37517 to plugin_formatter.

* Simplifying list test.

* Redistribute imports
acozine pushed a commit to acozine/ansible that referenced this pull request Jul 10, 2018
…ible#40212)

* Also marking non-string defaults.

* Adding list filter from ansible#37517 to plugin_formatter.

* Simplifying list test.

* Redistribute imports

(cherry picked from commit 0752dc1)
mattclay pushed a commit that referenced this pull request Jul 17, 2018
#42075)

* Documentation: show non-string non-iterable defaults for choices (#40212)

* Also marking non-string defaults.

* Adding list filter from #37517 to plugin_formatter.

* Simplifying list test.

* Redistribute imports

(cherry picked from commit 0752dc1)

* for 2.6 compatibility, removes dependency on collections.py, take two

* fix blank line error
@ansibot ansibot removed needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Jul 25, 2018
@ansibot

This comment has been minimized.

@ansibot ansibot added the ci_verified Changes made in this PR are causing tests to fail. label Jul 25, 2018
@@ -0,0 +1 @@
shippable/posix/ci/group1
Copy link
Member

Choose a reason for hiding this comment

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

shippable/posix/group1

So Jinja2 only has tests for testing string types, or iterable types,
which is quite limiting. If you need to test a boolean value in Jinja2
the only option is to test it is not a string and convert it to a string
and compare it to 'True' or 'False'.

So these tests are essential constructs in playbooks and templates.

This PR includes integration tests to validate Jinja2 behaviour.
@ansibot ansibot removed ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jul 25, 2018
Copy link
Member

@webknjaz webknjaz left a comment

Choose a reason for hiding this comment

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

Let's not tolerate copy-paste driven development



# NOTE: The existing Jinja2 'sequence' test also matches strings and dictionaries
def test_list(value):
Copy link
Member

Choose a reason for hiding this comment

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

This code is already available in ansible.module_utils.common.collections.is_sequence for broader use.
So please follow earlier suggestion to reuse it: #37517 (comment)

@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Jul 31, 2018
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Aug 8, 2018
@ansibot ansibot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Aug 24, 2018
@bcoca
Copy link
Member

bcoca commented Mar 13, 2020

So instead of duplicating tests to allow older versions of Jinja2 to work the same, but mask changes/implementation details on newer ones, we've opted to add a 'jinja2' collection with these kind of tests/filters that are missing from older versions. temporarily at https://galaxy.ansible.com/sivel/jinja2

@bcoca bcoca closed this Mar 13, 2020
@ansible ansible locked and limited conversation to collaborators Apr 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.6 This issue/PR affects Ansible v2.6 feature This issue/PR relates to a feature request. needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants