OpenImageIO icon indicating copy to clipboard operation
OpenImageIO copied to clipboard

Convert documentation examples into testsuite tests

Open lgritz opened this issue 2 years ago • 26 comments

The documentation contains lots of code examples embedded in the text. Some of them have bugs or are incorrect in various ways, either because they were never correct (having been typed in for the docs themselves and never directly tested), or were once correct but now the APIs have changed and the example was never updated to match.

To combat this, we would like to go through all the documentation, and to the greatest extent possible, convert every significant code example in the docs into a test -- that is, into actual code that will compile and run every time we run the testsuite -- and then the documentation will merely include the test code by reference, so they can never get out of sync again.

Explanation: https://github.com/OpenImageIO/oiio/wiki/Converting-documentation-examples-to-tests

There are dozens and dozens of code examples in the docs that we could do this with. Small steps -- do this for one or two at a time, that's fine.

lgritz avatar Sep 21 '23 05:09 lgritz

I'm not going to formally assign the issue to anyone, because no one's piece of it is going to close the issue (until there are literally no more examples left to convert in the docs). This will take probably dozens of PRs (and people, maybe) before it's truly complete.

Please feel free to work on it. Maybe leave a comment here saying which particular example(s) you want to do, so that others who also want to contribute to the task won't pick the same specific bit that you did.

lgritz avatar Sep 21 '23 16:09 lgritz

I would recommend that anybody wanting to leave a note here declaring that they want to work on an example pick ONE specific code example to start with, and drive that to a complete accepted PR, before declaring a whole section or chapter off-limits to others. Get the hang of the process with something small, figure out how rapidly you can do the conversions, and then for round 2, you can reserve a larger section that might be a few days of work for you.

lgritz avatar Sep 24 '23 16:09 lgritz

Hi @lgritz, I'm interested in picking up the conversion for the examples within imagebufalgo.rst as part of Dev Days. As a OIIO newbie, it seems like an excellent opportunity to get to know the API better!

buddly27 avatar Oct 11 '23 01:10 buddly27

That's great. It's a big chapter with a lot of example, maybe you want to claim a particular section of that chapter to start with?

lgritz avatar Oct 11 '23 01:10 lgritz

I can start with common principles and move to pattern generation if it goes smoothly. Unless someone else wants to get it :)

buddly27 avatar Oct 11 '23 01:10 buddly27

You claimed it first, it's yours!

I think that for examples where in the docs we show images, what we'll want to do is also have the docs switch to actually using the reference images from the examples. If that seems like too much to do all at once, you don't have to do it all in one step. We can move the code first, then change the location of the images.

lgritz avatar Oct 11 '23 02:10 lgritz

Hi @buddly27 and @lgritz... I'm on for dev days as well and new to the source. I'll take ImageBuf: Image Buffers to start. It has 3 (and a half) examples. After that, I could grab a section of imagebufalgo or perhaps another doc area that has an example1() template stub...

@lgritz On the half example... The last "example" has an elipsis section by design ("same as above") ... It is used to illustrate an option for code abstraction... Should I assume this should best be positioned in the repository with the full example?

@lgritz On reference images, do you mean the hrefs in the documentation rst markdown that point to generated images that live in this folder or this folder

Thanks

MichaelRCarroll-Intel avatar Oct 11 '23 23:10 MichaelRCarroll-Intel

Sounds great. Start with that chapter, see how it goes. (Sometimes fooling around with the docs and the testsuite can take longer than expected.)

The ellipsis is just to make the code example not be too long by mostly repeating what is directly above it. I suppose that in that final example, it didn't need to show make_black_impl at all. Looking at the examples I did in the other chapter, you should be able to see how to use the :start-after: and :end-before: to clip particular regions to appear in the docs. So the testsuite code can be a full working example, with only the key section actually appearing in the docs.

Yeah, by reference images, I mean in the ref/ directory of each test are the sample output that is the standard for the test (i.e., "if we run the test and it matches the reference output, the test passes; if it differs, the test fails"). If the docs are clarified by showing the image, than we may also reference the image in the docs themselves, but not all code examples are clarified by also showing specific image output.

lgritz avatar Oct 12 '23 00:10 lgritz

Hi @lgritz @MichaelRCarroll-Intel @buddly27 and everyone else planning to take/already working on a slice of this mega task or just wanting to browse the examples! I prepared a GitHub Project to track who's taking which example on OIIO docs examples to tests syncing for #3992 | Github Project and the overall progress. Since people will have different times and different sizes for this contribution, I made one for each example I could find. Will try to keep it updated with what you guys are doing.

If possible, when someone is picking up a task or made progress on one or there's an example not in this list and you want to add it, ping me to add it to this project, and I'll update it asap.

CheeksTheGeek avatar Oct 12 '23 03:10 CheeksTheGeek

For Dev Days, I'll start working on ImageOutput! Since the first two, Simple Write and Writing Scanline were covered in the Prototype PR, I'll continue from Write Individual Tiles

CheeksTheGeek avatar Oct 12 '23 04:10 CheeksTheGeek

Good morning everyone! I started a draft PR with one converted example to ensure we are all on the same page. I will update it and rebase my commit with the remaining examples from the "imagebufalgo" section throughout the day. Please let me know if you spot any issues!

@lgritz Would you prefer the PR to target an intermediate branch to ease collaborative work, or is it fine to target master?

buddly27 avatar Oct 12 '23 16:10 buddly27

Hi, @buddly27 for your testsuite/docs-examples-cpp/out.txt is there an OK just before it?, that's causing a test failure for the whole of docs-examples-cpp test when I was trying to test my local test additions.

OK
Comparing "simple.tif" and "ref/simple.tif"
PASS
Comparing "scanlines.tif" and "ref/scanlines.tif"
PASS

@lgritz it looks to be coming from a completely unrelated test at imageinout_test's function test_write_unwritable

CheeksTheGeek avatar Oct 12 '23 18:10 CheeksTheGeek

Hi @CheeksTheGeek, I don't see the error, but maybe I'm doing something wrong. I'm running the tests using ctest to be more granular:

OpenImageIO_ROOT=$(pwd)/dist LD_LIBRARY_PATH=$(pwd)/dist/lib/:$(pwd)/ext/dist/lib/ \
PYTHONPATH=$(pwd)/build/lib/python/site-packages/ \
ctest --test-dir ./build/ -R docs-examples- -VV

buddly27 avatar Oct 12 '23 19:10 buddly27

I do it using

  ~/projects/oiio-build ❯ ctest docs-examples-cpp --rerun-failed --output-on-failure    5s  base  14:46:24
Test project /Users/$USER/projects/oiio-build
    Start 2: docs-examples-cpp
1/1 Test #2: docs-examples-cpp ................***Failed    2.91 sec
Comparing "simple.tif" and "ref/simple.tif"
PASS
Comparing "scanlines.tif" and "ref/scanlines.tif"
PASS
-rw-r--r--@ 1 $USER  staff  107 Oct 12 15:54 out.txt
-rw-r--r--@ 1 $USER  staff  104 Sep 27 19:02 ref/out.txt
command = cmake /Users/$USER/projects/oiio/testsuite/docs-examples-cpp -DCMAKE_BUILD_TYPE=Release >> build.txt 2>&1 ;
cmake --build . --config Release >> build.txt 2>&1 ;
./docs-examples-imageioapi >> out.txt  ;
./docs-examples-imageoutput >> out.txt  ;
./docs-examples-imageinput >> out.txt  ;
./docs-examples-writingplugins >> out.txt  ;
./docs-examples-imagecache >> out.txt  ;
./docs-examples-texturesys >> out.txt  ;
./docs-examples-imagebuf >> out.txt  ;
./docs-examples-imagebufalgo >> out.txt  ;

#### Error: this command failed:  cmake --build . --config Release >> build.txt 2>&1
FAIL
comparisons are ['ref/simple.tif']
comparing simple.tif to ref/simple.tif
PASS: simple.tif matches ref/simple.tif
comparisons are ['ref/scanlines.tif']
comparing scanlines.tif to ref/scanlines.tif
PASS: scanlines.tif matches ref/scanlines.tif
comparisons are ['ref/out.txt']
comparing out.txt to ref/out.txt
Diff out.txt vs ref/out.txt was:
-------
--- out.txt     Thu Oct 12 15:54:44 2023
+++ ref/out.txt Wed Sep 27 19:02:06 2023
@@ -1,4 +1,3 @@
-OK
 Comparing "simple.tif" and "ref/simple.tif"
 PASS
 Comparing "scanlines.tif" and "ref/scanlines.tif"

NO MATCH for out.txt
FAIL out.txt
-----out.txt----->
OK
Comparing "simple.tif" and "ref/simple.tif"
PASS
Comparing "scanlines.tif" and "ref/scanlines.tif"
PASS
<----------
-----ref/out.txt----->
Comparing "simple.tif" and "ref/simple.tif"
PASS
Comparing "scanlines.tif" and "ref/scanlines.tif"
PASS
<----------
Diff was:
-------
--- out.txt     Thu Oct 12 15:54:44 2023
+++ ref/out.txt Wed Sep 27 19:02:06 2023
@@ -1,4 +1,3 @@
-OK
 Comparing "simple.tif" and "ref/simple.tif"
 PASS
 Comparing "scanlines.tif" and "ref/scanlines.tif"



0% tests passed, 1 tests failed out of 1

Total Test time (real) =   2.94 sec

The following tests FAILED:
          2 - docs-examples-cpp (Failed)
Errors while running CTest

CheeksTheGeek avatar Oct 12 '23 19:10 CheeksTheGeek

@lgritz Would you prefer the PR to target an intermediate branch to ease collaborative work, or is it fine to target master?

Target master is fine. I'll backport to the 2.5 branch later.

lgritz avatar Oct 12 '23 23:10 lgritz

@CheeksTheGeek Sorry for the late reply, as discussed in the slack channel, it might be some linking issue while attempting to run ctest outside of the wrapper (make test TEST=doc should hopefully work fine).

@lgritz I think #4016 is ready for review. I can tackle the next section separately to prevent overloading the review process!

buddly27 avatar Oct 13 '23 02:10 buddly27

Hello everyone! I'm hopping in on this issue and hoping to tackle some ImageInput test cases, starting with SimpleRead.

I'm new to the source and will be sure to reach out on Slack with more questions but wanted to get in on this discussion to help with planning.

CalvinSch avatar Oct 13 '23 17:10 CalvinSch