allow passing -api parameters to exiftool

[barasher]

Hi, thanks for the PR. Is it possible to add an integration test that use this option and that would be used as a non regression test ?

[Blesmol]

HI @barasher ,

I can try, although the only exiftool api option that I use so far is QuickTimeUTC, and I need to see on how a proper integration test for this could look like. Do you have an example for other integration tests within go-exiftool?

[barasher]

In fact, when I add a new option, I try to check if it effectively does what it is supposed to do. Since it is not possible to check every API parameter values, I would just check one. I would check on a specific file that a specific field is extracted with a specific API parameter value and that it isn't without the API parameter. It would also indirectly check that :

• the API parameter is still compatible with the JSON serialization
• there is no breaking change on exiftool with its API parameter

[barasher]

Have a look at the TestExtractAllBinaryMetadata test:

• it checks extraction without the exiftool parameter (the field X is not extracted)
• it check extraction with the exiftool parameter (the field X is now extracted)

[Blesmol]

Sorry for the delay, currently only able to check this every couple of days.

Some open questions on my side:

• You're bringing TestExtractAllBinaryMetadata as example. But isn't that a regression test, although initially you were mentioning non-regression tests? How do you differentiate here between both? In general I can add something like this. Need to see whether I can provide a binary testfile that is as small as possible.
• Which JSON serialization? Happy to test this as well, but wasn't aware that there is some JSON serialization anywhere.
• The part about "no breaking change on exiftool with its API parameter" I did not understand...

[barasher]

You're bringing TestExtractAllBinaryMetadata as example. But isn't that a regression test, although initially you were mentioning non-regression tests? How do you differentiate here between both? In general I can add something like this. Need to see whether I can provide a binary testfile that is as small as possible.

The idea of this test is to check that the Api function is concretely working. The best way to test it is to try it with a concrete sample that returns something with the Api settings that exiftools does not return without it. It proves that it works at least in one use-case.

Here is a sample with an existing file in the repository :

barasher@linux:~/go/src/github.com/barasher/go-exiftool/testdata$ exiftool 20190404_131804.jpg | grep 'File Block Size'

barasher@linux:~/go/src/github.com/barasher/go-exiftool/testdata$ exiftool -api systemtags 20190404_131804.jpg | grep 'File Block Size'
File Block Size                 : 4096

I call it a non regression test because if anything changes with the exiftool's -api parameter (for instance, -api is renamed -api_parameter, this unit-test should not work anymore. In go-exiftool's point of view, if something that used to work does not work anymore, it is a regression. The test checks that there is not any regression concerning the parameter.

I think that we agree and that it is just a vocabulary issue :)

---

Which JSON serialization? Happy to test this as well, but wasn't aware that there is some JSON serialization anywhere.

go-exiftool uses internally exiftool's -j parameter that formats metadata in JSON:

barasher@linux:~/go/src/github.com/barasher/go-exiftool/testdata$ exiftool 20190404_131804.jpg
Orientation                     : Rotate 90 CW
X Resolution                    : 72
Y Resolution                    : 72

barasher@linux:~/go/src/github.com/barasher/go-exiftool/testdata$ exiftool -j 20190404_131804.jpg
[{
  "Orientation": "Rotate 90 CW",
  "XResolution": 72,
  "YResolution": 72
}]

I feared that exiftools -api parameter could not be compatible with the -j parameter but it seems that it's ok:

barasher@linux:~/go/src/github.com/barasher/go-exiftool/testdata$ exiftool -j -api systemtags 20190404_131804.jpg 
[{
  "FileBlockSize": 4096
}]

So, this should be OK with go-exiftool

[barasher]

@Blesmol, are you still OK to work on this PR ?

[Blesmol]

@barasher Yep. Sorry for the delay, Newborn child in the family takes up a lot of time :) Will see that I find time within the next couple of days (after getting some proper sleep)

[Blesmol]

I think that we agree and that it is just a vocabulary issue :)

I agree :) What you described there I would call a regression test. The "non regression" part was what was confusing me in the wording.

I've added now another unittest that tests an actual api setting and shows that the parameter has an impact. Would that be sufficient? If I understood you correctly, then for the JSON part no additional tests are required at the moment?

[barasher]

Congratulations for your newborn baby ! Thanks for the PR, I'll certainly review and merge it this weekend !

[codecov-commenter]

Codecov Report

Merging #59 (a8df253) into master (70f0835) will increase coverage by 0.83%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #59      +/-   ##
==========================================
+ Coverage   79.71%   80.54%   +0.83%     
==========================================
  Files           2        2              
  Lines         281      293      +12     
==========================================
+ Hits          224      236      +12     
  Misses         40       40              
  Partials       17       17              

Impacted Files	Coverage Δ
exiftool.go	72.33% <100.00%> (+1.71%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 70f0835...a8df253. Read the comment docs.