From 9de2be4d9621ee2e37fe6302d088871a65fdd748 Mon Sep 17 00:00:00 2001 From: Jack Smith Date: Thu, 5 Oct 2023 10:02:27 -0400 Subject: [PATCH] Improve second-pass result checking by also directly checking the original file against the final expected result --- .../java/com/hubspot/jinjava/EagerTest.java | 64 +++++++++++++++++++ .../jinjava/ExpectedTemplateInterpreter.java | 63 +++++++++++++----- .../lib/tag/eager/EagerExtendsTagTest.java | 9 +++ .../eager/handles-cycle-in-deferred-for.jinja | 5 +- ...onstructs-map-node.expected.expected.jinja | 2 - .../reconstructs-map-node.expected.jinja | 8 +-- .../eager/reconstructs-map-node.jinja | 3 +- 7 files changed, 128 insertions(+), 26 deletions(-) diff --git a/src/test/java/com/hubspot/jinjava/EagerTest.java b/src/test/java/com/hubspot/jinjava/EagerTest.java index 490e2aa98..98960a4a4 100644 --- a/src/test/java/com/hubspot/jinjava/EagerTest.java +++ b/src/test/java/com/hubspot/jinjava/EagerTest.java @@ -551,6 +551,9 @@ public void itSetsMultipleVarsDeferredInChildSecondPass() { expectedTemplateInterpreter.assertExpectedOutput( "sets-multiple-vars-deferred-in-child.expected" ); + expectedTemplateInterpreter.assertExpectedNonEagerOutput( + "sets-multiple-vars-deferred-in-child.expected" + ); } @Test @@ -657,6 +660,8 @@ public void itEagerlyDefersMacro() { @Test public void itEagerlyDefersMacroSecondPass() { + localContext.put("foo", "I am foo"); + localContext.put("bar", "I am bar"); localContext.put("deferred", true); expectedTemplateInterpreter.assertExpectedOutput("eagerly-defers-macro.expected"); expectedTemplateInterpreter.assertExpectedNonEagerOutput( @@ -801,6 +806,9 @@ public void itHandlesDeferredImportVarsSecondPass() { expectedTemplateInterpreter.assertExpectedOutput( "handles-deferred-import-vars.expected" ); + expectedTemplateInterpreter.assertExpectedNonEagerOutput( + "handles-deferred-import-vars.expected" + ); } @Test @@ -822,6 +830,10 @@ public void itHandlesDeferredFromImportAsSecondPass() { expectedTemplateInterpreter.assertExpectedOutput( "handles-deferred-from-import-as.expected" ); + // Aliasing from macros is not yet supported in default execution + // expectedTemplateInterpreter.assertExpectedNonEagerOutput( + // "handles-deferred-from-import-as.expected" + // ); } @Test @@ -889,6 +901,9 @@ public void itHandlesDeferredInNamespaceSecondPass() { expectedTemplateInterpreter.assertExpectedOutput( "handles-deferred-in-namespace.expected" ); + expectedTemplateInterpreter.assertExpectedNonEagerOutput( + "handles-deferred-in-namespace.expected" + ); } @Test @@ -918,6 +933,9 @@ public void itHandlesBlockSetInDeferredIfSecondPass() { expectedTemplateInterpreter.assertExpectedOutput( "handles-block-set-in-deferred-if.expected" ); + expectedTemplateInterpreter.assertExpectedNonEagerOutput( + "handles-block-set-in-deferred-if.expected" + ); } @Test @@ -1001,6 +1019,9 @@ public void itHandlesDoubleImportModification() { @Test public void itHandlesDoubleImportModificationSecondPass() { interpreter.getContext().put("deferred", false); + expectedTemplateInterpreter.assertExpectedOutput( + "handles-double-import-modification.expected" + ); expectedTemplateInterpreter.assertExpectedNonEagerOutput( "handles-double-import-modification.expected" ); @@ -1016,6 +1037,9 @@ public void itHandlesSameNameImportVar() { @Test public void itHandlesSameNameImportVarSecondPass() { interpreter.getContext().put("deferred", "resolved"); + expectedTemplateInterpreter.assertExpectedOutput( + "handles-same-name-import-var.expected" + ); expectedTemplateInterpreter.assertExpectedNonEagerOutput( "handles-same-name-import-var.expected" ); @@ -1069,6 +1093,9 @@ public void itFullyDefersFilteredMacroSecondPass() { expectedTemplateInterpreter.assertExpectedOutput( "fully-defers-filtered-macro.expected" ); + expectedTemplateInterpreter.assertExpectedNonEagerOutput( + "fully-defers-filtered-macro.expected" + ); } @Test @@ -1103,6 +1130,7 @@ public void itReconstructsMapNode() { @Test public void itReconstructsMapNodeSecondPass() { interpreter.getContext().put("deferred", "resolved"); + expectedTemplateInterpreter.assertExpectedOutput("reconstructs-map-node.expected"); expectedTemplateInterpreter.assertExpectedNonEagerOutput( "reconstructs-map-node.expected" ); @@ -1123,6 +1151,9 @@ public void itDefersCallTagWithDeferredArgument() { @Test public void itDefersCallTagWithDeferredArgumentSecondPass() { interpreter.getContext().put("deferred", "resolved"); + expectedTemplateInterpreter.assertExpectedOutput( + "defers-call-tag-with-deferred-argument.expected" + ); expectedTemplateInterpreter.assertExpectedNonEagerOutput( "defers-call-tag-with-deferred-argument.expected" ); @@ -1145,6 +1176,9 @@ public void itHandlesHigherScopeReferenceModification() { @Test public void itHandlesHigherScopeReferenceModificationSecondPass() { interpreter.getContext().put("deferred", "b"); + expectedTemplateInterpreter.assertExpectedOutput( + "handles-higher-scope-reference-modification.expected" + ); expectedTemplateInterpreter.assertExpectedNonEagerOutput( "handles-higher-scope-reference-modification.expected" ); @@ -1181,6 +1215,9 @@ public void itDoesNotOverrideImportModificationInFor() { @Test public void itDoesNotOverrideImportModificationInForSecondPass() { interpreter.getContext().put("deferred", "resolved"); + expectedTemplateInterpreter.assertExpectedOutput( + "does-not-override-import-modification-in-for.expected" + ); expectedTemplateInterpreter.assertExpectedNonEagerOutput( "does-not-override-import-modification-in-for.expected" ); @@ -1196,6 +1233,9 @@ public void itHandlesDeferredForLoopVarFromMacro() { @Test public void itHandlesDeferredForLoopVarFromMacroSecondPass() { interpreter.getContext().put("deferred", "resolved"); + expectedTemplateInterpreter.assertExpectedOutput( + "handles-deferred-for-loop-var-from-macro.expected" + ); expectedTemplateInterpreter.assertExpectedNonEagerOutput( "handles-deferred-for-loop-var-from-macro.expected" ); @@ -1225,6 +1265,9 @@ public void itReconstructsNamespaceForSetTagsUsingPeriod() { @Test public void itReconstructsNamespaceForSetTagsUsingPeriodSecondPass() { interpreter.getContext().put("deferred", "resolved"); + expectedTemplateInterpreter.assertExpectedOutput( + "reconstructs-namespace-for-set-tags-using-period.expected" + ); expectedTemplateInterpreter.assertExpectedNonEagerOutput( "reconstructs-namespace-for-set-tags-using-period.expected" ); @@ -1241,6 +1284,9 @@ public void itUsesUniqueMacroNames() { public void itUsesUniqueMacroNamesSecondPass() { interpreter.getContext().put("deferred", "resolved"); expectedTemplateInterpreter.assertExpectedOutput("uses-unique-macro-names.expected"); + expectedTemplateInterpreter.assertExpectedNonEagerOutput( + "uses-unique-macro-names.expected" + ); } @Test @@ -1298,6 +1344,9 @@ public void itKeepsMacroModificationsInScopeSecondPass() { expectedTemplateInterpreter.assertExpectedOutput( "keeps-macro-modifications-in-scope.expected" ); + expectedTemplateInterpreter.assertExpectedNonEagerOutput( + "keeps-macro-modifications-in-scope.expected" + ); } @Test @@ -1310,6 +1359,9 @@ public void itDoesNotReconstructVariableInWrongScope() { @Test public void itDoesNotReconstructVariableInWrongScopeSecondPass() { interpreter.getContext().put("deferred", true); + expectedTemplateInterpreter.assertExpectedOutput( + "does-not-reconstruct-variable-in-wrong-scope.expected" + ); expectedTemplateInterpreter.assertExpectedNonEagerOutput( "does-not-reconstruct-variable-in-wrong-scope.expected" ); @@ -1356,6 +1408,9 @@ public void itRreconstructsValueUsedInDeferredImportedMacroSecondPass() { expectedTemplateInterpreter.assertExpectedOutput( "reconstructs-value-used-in-deferred-imported-macro.expected" ); + expectedTemplateInterpreter.assertExpectedNonEagerOutput( + "reconstructs-value-used-in-deferred-imported-macro.expected" + ); } @Test @@ -1371,6 +1426,9 @@ public void itAllowsDeferredLazyReferenceToGetOverriddenSecondPass() { expectedTemplateInterpreter.assertExpectedOutput( "allows-deferred-lazy-reference-to-get-overridden.expected" ); + expectedTemplateInterpreter.assertExpectedNonEagerOutput( + "allows-deferred-lazy-reference-to-get-overridden.expected" + ); } @Test @@ -1411,6 +1469,9 @@ public void itReconstructsNestedValueInStringRepresentationSecondPass() { expectedTemplateInterpreter.assertExpectedOutput( "reconstructs-nested-value-in-string-representation.expected" ); + expectedTemplateInterpreter.assertExpectedNonEagerOutput( + "reconstructs-nested-value-in-string-representation.expected" + ); } @Test @@ -1425,6 +1486,9 @@ public void itDefersLoopSettingMetaContextVar() { public void itDefersLoopSettingMetaContextVarSecondPass() { interpreter.getContext().put("deferred", "resolved"); interpreter.getContext().getMetaContextVariables().add("content"); + expectedTemplateInterpreter.assertExpectedOutput( + "defers-loop-setting-meta-context-var.expected" + ); expectedTemplateInterpreter.assertExpectedNonEagerOutput( "defers-loop-setting-meta-context-var.expected" ); diff --git a/src/test/java/com/hubspot/jinjava/ExpectedTemplateInterpreter.java b/src/test/java/com/hubspot/jinjava/ExpectedTemplateInterpreter.java index 83daa4e1b..4f6748344 100644 --- a/src/test/java/com/hubspot/jinjava/ExpectedTemplateInterpreter.java +++ b/src/test/java/com/hubspot/jinjava/ExpectedTemplateInterpreter.java @@ -47,34 +47,65 @@ public String assertExpectedOutputNonIdempotent(String name) { } public String assertExpectedNonEagerOutput(String name) { - JinjavaInterpreter preserveInterpreter = new JinjavaInterpreter( - jinjava, - jinjava.getGlobalContextCopy(), - JinjavaConfig - .newBuilder() - .withExecutionMode(DefaultExecutionMode.instance()) - .withNestedInterpretationEnabled(true) - .withLegacyOverrides( - LegacyOverrides.newBuilder().withUsePyishObjectMapper(true).build() - ) - .withMaxMacroRecursionDepth(20) - .withEnableRecursiveMacroCalls(true) - .build() - ); + String output; try { + JinjavaInterpreter preserveInterpreter = new JinjavaInterpreter( + jinjava, + jinjava.getGlobalContextCopy(), + JinjavaConfig + .newBuilder() + .withExecutionMode(DefaultExecutionMode.instance()) + .withNestedInterpretationEnabled(true) + .withLegacyOverrides( + LegacyOverrides.newBuilder().withUsePyishObjectMapper(true).build() + ) + .withMaxMacroRecursionDepth(20) + .withEnableRecursiveMacroCalls(true) + .build() + ); JinjavaInterpreter.pushCurrent(preserveInterpreter); preserveInterpreter.getContext().putAll(interpreter.getContext()); String template = getFixtureTemplate(name); - String output = JinjavaInterpreter.getCurrent().render(template); + output = JinjavaInterpreter.getCurrent().render(template); assertThat(JinjavaInterpreter.getCurrent().getContext().getDeferredNodes()) .as("Ensure no deferred nodes were created") .isEmpty(); assertThat(output.trim()).isEqualTo(expected(name).trim()); - return output; } finally { JinjavaInterpreter.popCurrent(); } + if (name.contains(".expected")) { + String originalName = name.replace(".expected", ""); + try { + JinjavaInterpreter preserveInterpreter = new JinjavaInterpreter( + jinjava, + jinjava.getGlobalContextCopy(), + JinjavaConfig + .newBuilder() + .withExecutionMode(DefaultExecutionMode.instance()) + .withNestedInterpretationEnabled(true) + .withLegacyOverrides( + LegacyOverrides.newBuilder().withUsePyishObjectMapper(true).build() + ) + .withMaxMacroRecursionDepth(20) + .withEnableRecursiveMacroCalls(true) + .build() + ); + JinjavaInterpreter.pushCurrent(preserveInterpreter); + + preserveInterpreter.getContext().putAll(interpreter.getContext()); + String template = getFixtureTemplate(originalName); + output = JinjavaInterpreter.getCurrent().render(template); + assertThat(JinjavaInterpreter.getCurrent().getContext().getDeferredNodes()) + .as("Ensure no deferred nodes were created") + .isEmpty(); + assertThat(output.trim()).isEqualTo(expected(name).trim()); + } finally { + JinjavaInterpreter.popCurrent(); + } + } + return output; } public String getFixtureTemplate(String name) { diff --git a/src/test/java/com/hubspot/jinjava/lib/tag/eager/EagerExtendsTagTest.java b/src/test/java/com/hubspot/jinjava/lib/tag/eager/EagerExtendsTagTest.java index a0040c750..945a2e9a4 100644 --- a/src/test/java/com/hubspot/jinjava/lib/tag/eager/EagerExtendsTagTest.java +++ b/src/test/java/com/hubspot/jinjava/lib/tag/eager/EagerExtendsTagTest.java @@ -54,6 +54,9 @@ public void itDefersBlockInExtendsChildSecondPass() { expectedTemplateInterpreter.assertExpectedOutput( "defers-block-in-extends-child.expected" ); + expectedTemplateInterpreter.assertExpectedNonEagerOutput( + "defers-block-in-extends-child.expected" + ); } @Test @@ -67,6 +70,9 @@ public void itDefersSuperBlockWithDeferredSecondPass() { expectedTemplateInterpreter.assertExpectedOutput( "defers-super-block-with-deferred.expected" ); + expectedTemplateInterpreter.assertExpectedNonEagerOutput( + "defers-super-block-with-deferred.expected" + ); } @Test @@ -83,6 +89,9 @@ public void itReconstructsDeferredOutsideBlockSecondPass() { expectedTemplateInterpreter.assertExpectedOutput( "reconstructs-deferred-outside-block.expected" ); + expectedTemplateInterpreter.assertExpectedNonEagerOutput( + "reconstructs-deferred-outside-block.expected" + ); } @Test diff --git a/src/test/resources/eager/handles-cycle-in-deferred-for.jinja b/src/test/resources/eager/handles-cycle-in-deferred-for.jinja index 75db4a6e2..9cbe73032 100644 --- a/src/test/resources/eager/handles-cycle-in-deferred-for.jinja +++ b/src/test/resources/eager/handles-cycle-in-deferred-for.jinja @@ -1,5 +1,8 @@ {% set foo = ['1','2','3'] %} +{% set one = '1' %} +{% set two = '2' %} +{% set three = '3' %} {%- for item in deferred %} {% cycle foo %} -{% cycle foo[0],foo[1],foo[2] %} +{% cycle one,two,three %} {%- endfor -%} diff --git a/src/test/resources/eager/reconstructs-map-node.expected.expected.jinja b/src/test/resources/eager/reconstructs-map-node.expected.expected.jinja index e66867c56..aea750573 100644 --- a/src/test/resources/eager/reconstructs-map-node.expected.expected.jinja +++ b/src/test/resources/eager/reconstructs-map-node.expected.expected.jinja @@ -1,5 +1,3 @@ -First key is foo. - foo ff bar bb ['resolved', 'resolved'] diff --git a/src/test/resources/eager/reconstructs-map-node.expected.jinja b/src/test/resources/eager/reconstructs-map-node.expected.jinja index 98b3f99fe..74c2a45a1 100644 --- a/src/test/resources/eager/reconstructs-map-node.expected.jinja +++ b/src/test/resources/eager/reconstructs-map-node.expected.jinja @@ -1,13 +1,11 @@ {% if deferred %} {% set foo = [fn:map_entry('foo', 'ff'), fn:map_entry('bar', 'bb')] %} {% endif %} -First key is {{ foo[0].key }}. -{% set my_list = [] %}{% for __ignored__ in [0] %}{% do my_list.append(deferred) %} -foo ff{% do my_list.append(deferred) %} -bar bb{% endfor %} +{% set my_list = [] %}{% for key, val in foo %}{% do my_list.append(deferred) %} +{{ key ~ ' ' ~ val }}{% endfor %} {{ my_list }} {% set my_list = [] %}{% for __ignored__ in [0] %}{% do my_list.append(deferred) %} foo{% do my_list.append(deferred) %} bar{% endfor %} -{{ my_list }} \ No newline at end of file +{{ my_list }} diff --git a/src/test/resources/eager/reconstructs-map-node.jinja b/src/test/resources/eager/reconstructs-map-node.jinja index 80acfeb80..4c95bd29b 100644 --- a/src/test/resources/eager/reconstructs-map-node.jinja +++ b/src/test/resources/eager/reconstructs-map-node.jinja @@ -2,8 +2,7 @@ {% if deferred %} {% set foo = {'foo': 'ff', 'bar': 'bb'}.items() %} {% endif %} -First key is {{ foo[0].key }}. -{% for key, val in {'foo': 'ff', 'bar': 'bb'}.items() -%} +{% for key, val in foo -%} {% do my_list.append(deferred) %} {{ key ~ ' ' ~ val }} {%- endfor %}