Add more tests for lambdas used in sections #47

Open
wants to merge 4 commits into
from

Projects

None yet

2 participants

@cjerdonek

Here are a few tests for increased coverage of the rules governing lambdas in the context of sections.

The Python implementation missed two of these test cases (the first and second) even though it passed all other tests involving lambdas or the single dot.

cjerdonek added some commits Apr 29, 2012
@cjerdonek cjerdonek Added two tests that lambdas short-circuit the section rendering rules. f5b2e9d
@cjerdonek cjerdonek Improved "Section - No Re-interpolation" test:
This change strengthens the test case.  In the previous version, if
the string "{{.}}" was on the top of the context stack, the test could
conceivably have passed with certain versions of bad code.
266747a
@cjerdonek cjerdonek Added a test that elements of a list of lambdas should each have thei…
…r results parsed.
493283c
@cjerdonek cjerdonek added a commit to cjerdonek/mustache-spec that referenced this pull request May 3, 2012
@cjerdonek cjerdonek Merge remote-tracking branch 'lambdas-section-short-circuit' into dev…
…elopment

See issue #47: mustache#47
6db2f0a
@pvande pvande commented on the diff May 24, 2012
specs/~lambdas.yml
@@ -107,6 +107,51 @@ tests:
template: "<{{#lambda}}-{{/lambda}}>"
expected: "<-Earth->"
+ - name: Section - Expansion of List Elements
@pvande
pvande May 24, 2012

Can you describe the thought behind this test?

I read it as:

  • lambdas is a list
  • {{#lambdas}} should iterate the list
  • If {{.}} for each iteration is a lambda:
    • It should be executed
    • It should receive the raw content string as an argument
    • The content of the iteration loop should be the return value from the lambda

This feels a bit off to me; nowhere else do we treat {{.}} specially. For the behavior you're describing, I'd expect to template {{#lambdas}}{{#.}}planet{{/.}}{{/lambdas}}.

@cjerdonek
cjerdonek May 25, 2012

I'm taking these responses one at a time. Thanks for taking a look at these and thinking about them, btw!

Can you describe the thought behind this test?

Sure. The purpose of this first test is basically to test the same thing as the following existing test--

- name: Section - Expansion
  desc: Lambdas used for sections should have their results parsed.
  ...
  template: "<{{#lambda}}-{{/lambda}}>"

but for a list of lambdas rather than a single lambda.

I'm interpreting the expected behavior for this case from this part of sections.yml:

If the data is not of a list type, it is coerced into a list as follows: if
the data is truthy (e.g. `!!data == true`), use a single-element list
containing the data, otherwise use an empty list.

For each element in the data list, the element MUST be pushed onto the
context stack, the section MUST be rendered, and the element MUST be popped
off the context stack.

combined with this part of ~lambdas.yml:

When used as the data value for a Section tag, the lambda MUST be treatable
as an arity 1 function, and invoked as such (passing a String containing the
unprocessed section contents).  The returned value MUST be rendered against
the current delimiters, then interpolated in place of the section.

The key observation (from the first excerpt above) is that a data element that is not a list is the same as a single-element list containing that data (because a non-list data type is first coerced into a list anyways).

In particular, the following existing test case:

- name: Section - Expansion
  desc: Lambdas used for sections should have their results parsed.
  data:
    planet: "Earth"
    lambda: !code
      python:  'lambda text: "%s{{planet}}%s" % (text, text)'
  template: "<{{#lambda}}-{{/lambda}}>"
  expected: "<-Earth->"

should be equivalent to the following, slightly-modified test case:

- name: Section - Expansion of Contents of Single-Element List
  desc: Lambdas used for sections should have their results parsed.
  data:
    planet: "Earth"
    lambdas:
      - !code
        python:  'lambda text: "%s{{planet}}%s" % (text, text)'
  template: "<{{#lambdas}}-{{/lambdas}}>"
  expected: "<-Earth->"

In other words, the lambda logic in the second excerpt above should be applied individually to each lambda in the list.

The test case I provided above was simply a two-element list instead of a one-element list.

@pvande
pvande May 25, 2012

This is a fascinating interpretation, and I love that you're investing the thought in this.

As mentioned in #46, the language around the handling of lambdas / methods in sections is not strictly accurate to the intent. In particular, the results of an arity 1 function call a) must be stringifiable and b) rendered against the context stack. This makes parts 5 and 6 completely non-applicable (okay, maybe not completely, but it does seem unlikely that you'll commonly find something useful to call on a rendered template string in a list context). The text then goes on to say that the returned data value should be "listified" if not already "listy", and that each thing in the list (i.e. just the rendered template) should be pushed onto the context stack.

The failure points in the current verbage (in sections.yml) seem to be:

  • The return result from an unary function call is described as "data".
  • Calling lambda.y seems to have useless semantics.
  • The return result should be pushed onto the context stack (and hence, never interpolated).

In practice, the desired (and codified) behavior of unary methods is the same as for lambdas: specifically, when encountered, they should be called with the raw content of the section (as per sections.yml, point 4), the returned value rendered against the current context stack and delimiters (as per ~lambdas.yml, ¶3), and the rendered value should be immediately substituted for the section tag (as per ~lambdas.yml, ¶3). Dotted name parts following such a function call have undefined behavior.

Furthermore, the "listification" of data elements doesn't happen until after name resolution (which includes function invocations) has completed. So while 'x': 'foo' and 'x': ['foo'] are equivalent for {{#x}}, 'x': lambda: 'foo' is equivalent to 'x': ['foo'], not 'x': [lambda: 'foo'].

I hope this helps clear things up. If not, please feel free to say so. :)

@pvande pvande commented on the diff May 24, 2012
specs/~lambdas.yml
@@ -107,6 +107,51 @@ tests:
template: "<{{#lambda}}-{{/lambda}}>"
expected: "<-Earth->"
+ - name: Section - Expansion of List Elements
+ desc: Lambdas used for sections in a list should each have their results parsed.
+ data:
+ planet: "Earth"
+ lambdas:
+ - !code
+ python: 'lambda text: "~{{%s}}~" % text'
+ - !code
+ python: 'lambda text: "#{{%s}}#" % text'
+ template: "<{{#lambdas}}planet{{/lambdas}}>"
+ expected: "<~Earth~#Earth#>"
+
+ - name: Section - Mixed Lists
@pvande
pvande May 24, 2012

This again comes back to special handling for {{.}}.

@cjerdonek
cjerdonek May 25, 2012

This is just a variation (i.e. slightly more complex version) of the test case above, so we can discuss this one after settling the above. Again, the key is to apply the same logic you would to a non-list data type, but for each element of the list sequentially.

@pvande pvande commented on the diff May 24, 2012
specs/~lambdas.yml
+ python: 'lambda text: "#{{%s}}#" % text'
+ template: "<{{#lambdas}}planet{{/lambdas}}>"
+ expected: "<~Earth~#Earth#>"
+
+ - name: Section - Mixed Lists
+ desc: Sections should permit mixed lists of lambdas and non-lambdas.
+ data:
+ planet: "Earth"
+ lambdas:
+ - !code
+ python: 'lambda text: "~{{%s}}~" % text'
+ - 1
+ template: "<{{#lambdas}}planet{{/lambdas}}>"
+ expected: "<~Earth~planet>"
+
+ - name: Section - Context Stack
@pvande
pvande May 24, 2012

This seems like a really useful test, but I'm having trouble reasoning about how it plays with the expectations we've set up.

Then again, if {{.}} from inside a lambda isn't useful, that seems like an oversight. Any idea what kind of implementation "tricks" are required to make this work?

@pvande pvande commented on the diff May 24, 2012
specs/~lambdas.yml
+ template: "<{{#lambdas}}planet{{/lambdas}}>"
+ expected: "<~Earth~planet>"
+
+ - name: Section - Context Stack
+ desc: |
+ Lambdas used for sections should not be pushed onto the context
+ stack before rendering their return value.
+ data:
+ planet: "Earth"
+ star: "Sun"
+ lambda: !code
+ python: 'lambda text: "~{{star}} %s {{.}}~" % text'
+ template: "<{{#planet}}{{#lambda}}&{{/lambda}}{{/planet}}>"
+ expected: "<~Sun & Earth~>"
+
+ - name: Section - No Re-interpolation
@pvande
pvande May 24, 2012

Absolutely. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment