Use Diff package instead diff program to test cabal-to-dhall

[jneira]

As commented in #72

[ocharles]

Can you confirm invalid tests produce useful output? Attaching example output here would help me evaluate.

[jneira]

The Diff output is similar to the utility one, for example modifying the simple.cabal test case with:

cabal-version: 2.0
name: test
version: 1.0
author: author

The output of the failing tests is:

Diff between expected golden-tests/cabal-to-dhall/simple.dhall and actual golden-tests/cabal-to-dhall/simple.cabal :
6c6
<         ""
---
>         "author"

Maybe the comment should be more precise cause is the difference with the dhall file generated from cabal one.

[jneira]

Anyway let me replicate the output for goldenVsStringDiff so the difference with previous version only would be the style of diff output (Diff output is like standard diff without the -u flag)

[quasicomputational]

Is there a reason why this code is written differently to how dhall-to-cabal's test is? I would have thought that the problems are the same (compare an abstract representation in memory to a serialised representation on disk), so the code ought to look the same.

[jneira]

@quasicomputational mmm, dhall-to-cabal uses the GenericPackageDescription parsed from both files to test so it not depends on formatting. To get the same behaviour for cabal-to-dhall, i think we should compare the generated dhall ast from both sources instead the pretty printed output text of cabalToDhall

[quasicomputational]

I think we should. dhall 1.15 is going to change the formatting of its output (see #77's diffstat for an indication of the damage), so if we can test for equality on the abstract trees and then diff on the concrete syntax it'd be good.

I agree with your earlier worry that the error message isn't clear that the diff is comparing to the re-serialised golden file rather than the exact representation on disk, but that's actually already a problem in the dhall-to-cabal test, so I guess it shouldn't block merging this and we can fix that later.

[jneira]

With the last commit the diff output is similar to goldenVsStringDiff

• Previous output:

  
  dhall-to-cabal-1.1.0.0: test (suite: golden-tests)

golden tests dhall-to-cabal SPDX: OK (17.51s) nested-conditions: OK (19.53s) gh-55: OK (11.00s) gh-53: OK (12.52s) empty-package: Diff between expected golden-tests/dhall-to-cabal/ empty-package.cabal and actual golden-tests/dhall-to-cabal/empty-package.dhall :

4a5

author: author

FAIL (0.89s) Generated .cabal file does not match input dhall-to-cabal: OK (54.37s) conditional-dependencies: OK (18.91s) compiler-options-order: OK (14.79s) cabal-to-dhall SPDX: OK (0.15s) simple: FAIL (0.03s) Test output was different from 'golden-tests/cabal-to-dhall/simple.dhall'. Output of ["diff","-u","golden-tests/cabal-to-dhall/simple.dhall","C:\TEMP\si mple.dhall5852-1.actual"]: --- golden-tests/cabal-to-dhall/simple.dhall 2018-06-05 14:47:20.5741 18800 +0200 +++ C:\TEMP\simple.dhall5852-1.actual 2018-06-06 09:20:00.875802800 +0 200 @@ -3,7 +3,7 @@ in let types = ../../dhall/types.dhall

   in  { author =
  -        ""
  +        "author"
       , benchmarks =
           [] : List { benchmark : types.Config → types.Benchmark, name : Te

xt }

       , bug-reports =
otheros:                  OK (0.04s)
gh-36:                    OK (0.06s)
conditional-dependencies: OK (0.04s)

2 out of 13 tests failed (149.88s)

dhall-to-cabal-1.1.0.0: Test suite golden-tests failed

* Actual output:

dhall-to-cabal-1.1.0.0: test (suite: golden-tests)

golden tests dhall-to-cabal SPDX: OK (18.01s) nested-conditions: OK (19.84s) gh-55: OK (11.38s) gh-53: OK (12.53s) empty-package: FAIL (0.92s) Test output was different from 'golden-tests/dhall-to-cabal/empty-package. cabal'. Output of diff between 'golden-tests/dhall-to-cabal/empty-package.cabal' a nd test output using 'golden-tests/dhall-to-cabal/empty-package.dhall': 4a5

author: author dhall-to-cabal: OK (55.44s) conditional-dependencies: OK (19.29s) compiler-options-order: OK (15.22s) cabal-to-dhall SPDX: OK simple: FAIL Test output was different from 'golden-tests/cabal-to-dhall/simple.dhall'.

  Output of diff between 'golden-tests/cabal-to-dhall/simple.dhall' and test

output using 'golden-tests/cabal-to-dhall/simple.cabal': 6c6 < ""

  >         "author"
otheros:                  OK
gh-36:                    OK (0.05s)
conditional-dependencies: OK (0.01s)

2 out of 13 tests failed (152.73s)

dhall-to-cabal-1.1.0.0: Test suite golden-tests failed

[jneira]

As the diff ouput can be useful although we change the test check test in cabal-to-dhall test group, i think we could include this version and later change the check to compare the dhall ast's

[jneira]

@ocharles what do you think about merge it as is?

[ocharles]

Hi - I've been at ZuriHac all weekend and haven't had a chance to review yet. I'll try and review it this week!

On Sun, 10 Jun 2018, 8:39 pm Javier Neira, notifications@github.com wrote:

@ocharles https://github.com/ocharles what do you think about merge it as is?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/dhall-lang/dhall-to-cabal/pull/90#issuecomment-396071334, or mute the thread https://github.com/notifications/unsubscribe-auth/AABRjtR-Q-7wKBTgZKNfPXm9TyM4VTJ5ks5t7WfjgaJpZM4UbbL5 .

[ocharles]

To get the same behaviour for cabal-to-dhall, i think we should compare the generated dhall ast from both sources instead the pretty printed output text of cabalToDhall

I think we should do this. https://hackage.haskell.org/package/dhall-1.14.0/docs/Dhall-Diff.html is already doing the hard work for us, it just didn't exist when we first wrote these tests.

[quasicomputational]

Looks sensible to me (still). I think I'd merge this without updating the tests to Dhall 1.19 and leave that to #143, but it's not really a big difference either way.

[jneira]

Ok, i've re-rebased this one with master

[ocharles]

Slightly reformatted but looks good to me! Will merge once CI is happy. Thank you!