search

pepijnve / ditaa

A stripped down version of ditaa.
GNU Lesser General Public License v3.0
10 stars 3 forks source link

Create unit test suite for existing test files #2

Open pepijnve opened 8 years ago

pepijnve commented 8 years ago

The project contains a number of test input files but doesn't have an automated test suite to actually test these. Create unit tests that at least try to process the files and check that no exception occur.

neolefty commented 8 years ago

Take a look at the latest version of ditaa/build/release.xml -- it can run the unit tests now, in particular VisualTester. There's also a new build.xml in ditaa/tests that can call it remotely:

$ git clone https://github.com/stathissideris/ditaa
$ cd ditaa/tests
$ ant generate test

The "generate" target followed by the "test" target is cheating though -- it generates "master" images, compares generates "test" images the same way and compares them. So it's no surprise they are identical :)

neolefty commented 8 years ago

I suppose the use case is:

  1. Generate reference images
  2. Make a change to the code
  3. Regenerate the images and check whether they changed

It's automatic if you want the images to not change, but manual if you're fixing a visual bug.

pepijnve commented 8 years ago

Oh right. Now I remember stripping out all of that stuff in fc9960fdf270d3c1fd5e58209965b70a247f6fbb :D

My goal here is simply to have something to run the code automatically with a bunch of inputs. Image comparison tests are extremely brittle and can only be run reliably on the same machine with the exact same Java runtime that was used to generate the master images. To solve that you need to start adding tolerances, etc; I would rather not go there...

neolefty commented 8 years ago

Ha ha, you'd end up with "why does this unit test depend on OpenCV?"

I agree the scope of that test is pretty narrow. It was already there, and I wanted an easy way to run it and look at the images, so I added it and the other unit tests to the ant release build.

The other half of it, triggered by "ant generate" is maybe more what you're looking for. It creates reference images from the text files.

My initial goal was simply to have something to run the code automatically with a bunch of inputs. Image comparison tests are extremely brittle and can only be run reliably on the same machine that generated the master images. To solve that you need to start adding tolerances, etc; I would rather not go there...

— Reply to this email directly or view it on GitHub https://github.com/pepijnve/ditaa/issues/2#issuecomment-178703019.

pepijnve commented 8 years ago

At this point the basic tests I have are sufficient. I might add something to dump the images based on a system property kind of like you described above.

My fork is starting to diverge quite a bit from Stathis' version already (massive code modernization/clean up mainly and stripping work-in-progress or unused functionality) so I'm only going to cherry pick upstream changes. I ditched the Ant script in favour of Gradle, so cherry picking build changes is not an option anymore... Thanks for the suggestions though.

neolefty commented 8 years ago

That sounds like just what Stathis' version needs. Did you ever talk with him about a merge? I'll have to look more at what you've done when I get a chance.

On Wed, Feb 3, 2016 at 2:01 AM, Pepijn Van Eeckhoudt < notifications@github.com> wrote:

At this point the basic tests I have are sufficient. I might add something to dump the images based on a system property kind of like you described above.

My fork is starting to diverge quite a bit from Stathis' version already (massive code modernization/clean up mainly and stripping work-in-progress or unused functionality) so I'm only going to cherry pick upstream changes. I ditched the Ant script in favour of Gradle, so cherry picking build changes is not an option anymore... Thanks for the suggestions though.

— Reply to this email directly or view it on GitHub https://github.com/pepijnve/ditaa/issues/2#issuecomment-178723287.

pepijnve commented 8 years ago

No I didn't, probably should have. The project looked pretty dead so I went ahead with a fork rather than trying to get changes accepted upstream. I would be happy to work towards merging this back upstream, but at this it's going to be quite an intimidating PR.

This fork started when I wanted to bundle ditaa with asciidoctor-diagram. Stathis' version had a hard dependency on Batik, but as far as I could tell that wasn't really used anywhere. If I remember correctly it was used for custom shapes (which you couldn't specify on the command line) and for unfinished SVG output. Since neither were useful for my needs so I stripped out the SVG code which allowed me to avoid having to bundle Batik (because it's huge).

Beyond that I've made the following changes