Introduce a dynamic test mechanism

[binaryseed]

Hi! This is a sweet project!

Here's an idea for a simple way of defining test cases as mentioned in #12

With this PR, you just add more "Cases" to any file in test/cases and they will be compared in their own test.

The test will be named like "test Preserve keyword list syntax (kw_list.exs)", so it would be easy to tell where the failure comes from...

Right now the "format" looks like this:

#=- CASE: Preserve comments
#=- BEFORE:
def hello(name) do
    "hello " <> name
end
#=- AFTER:
def hello(name) do
  "hello " <> name
end

Open to suggestions!

[uohzxela]

Hi @binaryseed! Thanks a lot for your contribution. Your idea sounds really neat, since having a file per test case could be cumbersome. Just wondering, does it work with comments as well? Some test cases have comments so it'd be great if you can do a quick sanity test for that as well. :)

@whatyouhide what do you think of this idea?

[binaryseed]

I added an example that shows that comments are preserved.

Also, I tweaked the formatting so the delimiters are more obvious, there's an explicit begin & end, and you can add optional "spacers" that will get ignored

[whatyouhide]

I am not sure about this as it, in my mind, complicates things a bit. Can I ask why we don't test this as:

before = """
def ...  do :ok end
"""

after = """
def ... do
  :ok
end
"""

assert format(before) == after

?

[lpil]

The method that whatyouhide is what I'm doing and I find it to be very easy to work with.

[lpil]

Huh, I didn't know you could do this. Pretty cool.

[binaryseed]

Yeah, this is simply what the test macro boils down to: https://github.com/elixir-lang/elixir/blob/master/lib/ex_unit/lib/ex_unit/case.ex#L277

[binaryseed]

You could certainly do that, but I think there are a few benefits to defining the cases outside of the tests:

• The number of test cases could reach in to many hundreds, and this reduces the boilerplate setup needed to create a new case (or make a tweak to all existing cases)
• Your editor / github will do proper syntax highliting on the code in the test/cases files, but not on the raw strings inside of tests.
• It's nice to look at the before/after code on its own, not surrounded by the test code.

[lpil]

https://github.com/lpil/exfmt/blob/master/test/exfmt/integration/def_test.exs

Seems about the same amount of boilerplate to me. :)

[binaryseed]

Cool, didn't know about your project @lpil ...

I'm not seeing a 'before' / 'after' case in those tests, is it just asserting that the input equals the output when run through the formatter? Do you have test cases that run "bad" input through and assert that it comes out correct?

I don't know about you but I really rely on syntax highlighting to quickly read code, and I have a hard time reading code inside raw strings...

[binaryseed]

Ahh, this is interesting: https://github.com/lpil/exfmt/blob/master/test/exfmt/integration/comments_test.exs#L84-L98

Looks like you did the before / after comparison by overriding the ~> operator.

That's quite nice and simple, but the highlighting issue still remains

  test "@spec" do
    """
    @spec run(String.t) :: atom | String.t | :hello
    """ ~> """
    @spec run(String.t)
          :: atom | String.t | :hello
    """
  end

vs:

#=- CASE: @spec
#=- BEFORE:
#=-
@spec run(String.t) :: atom | String.t | :hello
#=-
#=- AFTER:
#=-
@spec run(String.t)
      :: atom | String.t | :hello

[lpil]

After writing a few tests I decided that the before/after comparison is rarely useful as almost all formatting information is discarded by the parser.

[binaryseed]

Oh interesting! If that's the case, it seems like you could literally just have a series of files that have properly formatted code, run them through the formatter and expect no changes.

[lpil]

That would work, there's a few trade offs though:

• I find it easier to work with a few files containing lots of tests rather than lots of files.
• You'd have to re-implement tags for tests.
• You might want to split the files into batches and create a module per batch- large test modules can cause ExUnit to slow down a little.

On the flip side, less boilerplate and you get syntax highlighting. :)

[whatyouhide]

Consider that the string comparison approach is simpler to implement. I would say that that's the most important feature at this stage of the project, so personally I would start with that.

[binaryseed]

Cool, well I'll go ahead and close this for now then..

[uohzxela]

Thanks for the fruitful discussion, everyone. I think both approaches have their pros and cons though. Dynamic testing (test case as string) is useful for small examples but lacks syntax highlighting. Static testing (test case as file) is useful for integration tests but you have to tag the test cases from another place.

To be fair, @binaryseed's version of dynamic testing takes the best of two worlds without the disadvantages. The only downside is the need for careful delimiter handling inside the test file and a more complex implementation. But there are always tradeoffs.

@binaryseed, if the code is still present on your machine, could you please push your dynamic-case-definitions branch? We can use it as a reference in the future, should we decide to revamp our testing strategy. 😄

[binaryseed]

Yeah, I put the branch back up for the future!

[uohzxela]

Awesome @binaryseed, thanks a lot! ❤️