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

Tests, Add New assert_render helper #74

Merged
merged 1 commit into from
Jun 2, 2021

Conversation

Jcambass
Copy link
Contributor

@Jcambass Jcambass commented Jun 2, 2021

While implementing whitespace support I came to the conclusion that it makes sense to auto generate different whitespace trimming variants of a given template to gain a good test coverage of edge cases while keeping test maintainable.

One problem I encountered, was that since the templates where generated I didn't know the actual input that was used when comparing the render results of liquid and solid.

My solution was adding a new assert_render macro that renders the template in liquid and solid and then compares them (as we do already with cases_test), but then includes the passed template in the assertion output if there is an missmatch.

I'm trying to keep the main patch for the whitespace support as small and reviewable as possible.
Since this particular change can stand alone and IMHO makes also sense outside of the context of whitespace support I've opened this PR.

Sample output:

  1) test case 113 (:"Elixir.Solid.Integration.Cases.113Test")
     test/integration/cases_test.exs:17
     Render result was different!
     Input:
     {% if enabled? %}
     ON
     {% else %}
     OFF
     {% endif %}

     code:  liquid_output == solid_output
     left:  "\nON\n\n"
     right: "\nOFF\n\n"
     stacktrace:
       test/integration/cases_test.exs:21: (test)

While implementing [whitespace support](edgurgel#67) I came to the conclusion that it
makes sense to auto generate different whitespace trimming variants of
a given template to gain a good test coverage of edge cases while keeping test maintainable.

One problem I encountered, was that since the templates where generated
I didn't know the actual input that was used when comparing the render results of liquid and solid.

My solution was adding a new `assert_render` macro that renders the template in liquid and solid
and then compares them (as we do already with `cases_test`), but then includes the passed template in the assertion output if there is an missmatch.

I'm trying to keep the main patch for the whitespace support as small and revieable as possible.
Since this particular change can stand alone and IMHO makes also sense outside of the context of whitespace support I've opened this PR.

Sample output:
```
  1) test case 113 (:"Elixir.Solid.Integration.Cases.113Test")
     test/integration/cases_test.exs:17
     Render result was different!
     Input:
     {% if enabled? %}
     ON
     {% else %}
     OFF
     {% endif %}

     code:  liquid_output == solid_output
     left:  "\nON\n\n"
     right: "\nOFF\n\n"
     stacktrace:
       test/integration/cases_test.exs:21: (test)
```
@edgurgel
Copy link
Owner

edgurgel commented Jun 2, 2021

This is great! Thanks!

BTW the integration tests have a tag each so one can do this:

mix test --only case:013 and run just that case. Not sure if I documented this somewhere 🤔

@edgurgel edgurgel merged commit cd61bd6 into edgurgel:master Jun 2, 2021
@Jcambass
Copy link
Contributor Author

Jcambass commented Jun 2, 2021

@edgurgel Jup I figured that out (not really documented AFAIK).

To be honest, including the input in the assertion message isn't really critical for the cases tests but is quite critical for generated tests where you can't just look at the file to see the source. Glad that you still see it useful as a general thing :)

@Jcambass Jcambass deleted the add_assert_render branch June 2, 2021 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants