[Insight-developers] ITK v4: Best practice for adding test data?

[Brad King]

On 06/08/2011 10:21 AM, Cory Quammen wrote:
> What is the current best practice for contributing tests with new
> input and baseline images?
> 
> Here is what I have in mind for contributing a patch with new test data:
> 
> 1). Submit patch to gerrit for first round of feedback. Expect test
> failures on the robot builds for the new tests.
> 2). If gerrit reviews are positive, add test data to the ITK data git
> repository and push.
> 3). Request another round of builds on gerrit using "Request build: all"
> 4). Request another round of reviews. Merge if approved.
> 5). If not approved, back my changes out of the ITK data repository.
> 
> Is there a better procedure?

That's what folks have been doing.  Kitware will probably demo the new
ExternalData automagic data upload approach at the upcoming meeting.
The approach has very little intrusion on the standard development
workflow.  Here is a high-level summary:

  - Copy the new image into the local source tree directory
  - Reference the image from the itk_add_test call with a
    simple DATA{...} wrapper around the path to the file.
  - Run CMake, build, and test.
  - git add, git commit, git gerrit-push

Note that other than the DATA{} syntax this workflow is no different
from just adding the data file to the source tree with no submodule.

Here is how to try it now.  First make sure your development
environment is up to date:

(0) Checkout master and pull the latest upstream version.
    Run the developer setup scripts to make sure you have the
    latest version of the "git gerrit-push" alias.

Now follow the standard ITK development steps from here:

  http://www.itk.org/Wiki/ITK/Git/Develop#Create_a_Topic

During the "edit files" step, do this:

(1) Copy the new baseline image to a "Baseline" directory inside
    the module's "test" directory in your source tree, say
    "test/Baseline/MyTest.png".

(2) Write the itk_add_test call in the test/CMakeLists.txt file next
    to this Baseline directory.  Reference the image in the test command
    line with a relative path wrapped in DATA{...}:

      itk_add_test(NAME MyTest COMMAND ...
        --compare DATA{Baseline/MyTest.png} ...)

(3) Run cmake-gui on the build tree and use the "Add Entry" button
    to set "ExternalData_LINK_CONTENT" to a STRING with value "MD5".
    Configure and generate (then build and run the test).  During
    configuration you should see a message like

      Linked .../test/Baseline/MyTest.png.md5 to ExternalData MD5/...

    This means that CMake computed the hash of the data object,
    moved the original file out of the way, and left a "content link"
    in its place.

Returning to the standard instructions linked, above:

(4) Use "git add" to stage test/Baseline/MyTest.png.md5 and your
    changes locally for commit.  Then "git commit" them.  You should
    see another message from the pre-commit hook like

      Content link .../test/Baseline/MyTest.png.md5 -> .ExternalData/MD5/...

    This means that the pre-commit hook noticed what CMake did in the
    previous step, moved the real data file to another local place
    under the hood, and added a copy of it to a disjoint/hidden
    place in the local Git repository.

(5) Use "git gerrit-push" to send the topic to Gerrit for review as
    usual.  You can try "git gerrit-push --dry-run" the first time to
    see what would happen.  You should see output like this:

      *       ...:refs/data/commits/...     [new branch]
      *       HEAD:refs/for/master/my-topic  [new branch]
      Pushed refs/data and removed local copy:
        MD5/...

    This means that the script pushed your topic for review as usual.
    It also uploaded the data object behind the scenes to a disjoint
    and hidden place in Gerrit (after which a robot will move it to
    a more permanent location that ITK knows to check at build time).

Finally some cleanup from this demonstration:

(6) Run cmake-gui on the build tree again and use the "Remove Entry"
    button to take out the value added above since it is still
    experimental and will be enabled by a different mechanism
    later.

-Brad