mustache / spec

The Mustache spec.
MIT License
368 stars 71 forks source link

Add a test that dotted name resolution should make forward-only progress #48

Closed cjerdonek closed 12 years ago

cjerdonek commented 12 years ago

This test case tests that successive portions of dotted names should be resolved against former resolutions (i.e. forward-only progress).

I don't think a test like this currently exists. It seems like an important case.

pvande commented 12 years ago

Tests for this presently exist: https://github.com/mustache/spec/blob/master/specs/interpolation.yml#L151

cjerdonek commented 12 years ago

Here's more background on this one. This test case came up because someone submitted an incorrect implementation of dotted names to Pystache that passed all of the existing Mustache spec tests, but it didn't pass the test case I'm submitting here. Here is where it came up:

https://github.com/defunkt/pystache/pull/100#issuecomment-5480055

To illustrate, I just made a small change to the most current Pystache implementation (in a separate branch) that passes all of the existing spec tests -- but not the one I'm submitting. You can see it here:

https://github.com/cjerdonek/pystache/commit/5bab5b59500df5880d040ef996c4fc621d2439d5

It's hard to explain concisely, but what the case above is checking for is that if dotted name resolution fails at some point beyond the first dotted name part, then the implementation should terminate the name resolution process. The implementation should not proceed to try resolving the original full dotted name against later items in the context stack. The part of the spec that explains this is here (in the interpolation section):

5) If any name parts were retained in step 1, each should be resolved
against a context stack containing only the result from the former
resolution.  If any part fails resolution, the result should be considered
falsey, and should interpolate as the empty string.

As I said above, I think this is an important point and is easy for someone to get wrong if they're not careful.

pvande commented 12 years ago

Ah... So the failing implementation continues up the stack if resolution fails for any part of the dotted name, not just if the first part. Well, crap! That's the kind of thing that the "Dotted Names - Broken Chain Resolution" should be testing.

Is there a way we could write a test that proves that more succinctly?

cjerdonek commented 12 years ago

Thanks. Okay, I adjusted the test to be minimal in the commit above: e1ddf6048c99f5c81352eb04a84edc0669c4320b.

I also checked that this test passes in the newest Pystache code, and that it continues to be the only test that fails in the temporary branch with the incorrect implementation that I created above.

cjerdonek commented 12 years ago

Thanks for this!