Skip to content

testsuite: test the oiiotool 'Channel reordering and padding' doc examples - #5464

Open
linsen458-spec wants to merge 1 commit into
AcademySoftwareFoundation:mainfrom
linsen458-spec:test/doc-example-conversion
Open

linsen458-spec wants to merge 1 commit into
AcademySoftwareFoundation:mainfrom
linsen458-spec:test/doc-example-conversion

Conversation

@linsen458-spec

@linsen458-spec linsen458-spec commented Sep 10, 2026

Copy link
Copy Markdown

Part of #3992, reworked on top of the merged #5465.

This reworks the testsuite/oiiotool-channels test so that the oiiotool command lines from the "Channel reordering and padding" section of the docs live verbatim inside run_commands() blocks in the run.py, each bracketed by BEGIN/END-docs-channels-* marker comments. src/doc/oiiotool.md then literalincludes each command between its markers — so the docs and the test are literally the same lines and can no longer drift apart.

The setup images intentionally use asymmetric per-channel colors: with the default black/white checker pattern, R and B are identical in every pixel, which made the channel-swap and alpha-from-red examples produce outputs indistinguishable by content hash.

Testing: the content hashes of all six output images are pinned in out.txt (text-only comparison, no reference images needed), and the test passes locally.


This is not my native language, I'm using an LLM to restate and fix grammar in my replies. All the code was reviewed by me personally, and the description above was reviewed and approved by me as well. Looking forward to contributing more here, and happy to hear any feedback.

@linux-foundation-easycla

linux-foundation-easycla Bot commented Sep 10, 2026

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: linsen458-spec / name: linsen (94346c0)

@linsen458-spec
linsen458-spec force-pushed the test/doc-example-conversion branch 2 times, most recently from 994c08d to 94346c0 Compare September 10, 2026 13:00
@lgritz

lgritz commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

I think there might be a misunderstanding about what #3992 is asking for (outlined in more detail in the wiki page linked to on that issue).

The goal is:

  1. Make sure every code or command sample in the docs are actually executed, tested code, so that it's literally impossible for them to contain typos or mistakes or not do what they claim to do.
  2. Not allow the code shown in the docs to diverge from code in the tests, because they are literally the same lines, not duplicates in different files that could get out of sync.

The idea here is not merely to have tests from the examples represented in the testsuite (though that is one goal, but as a note, we do already test various oiiotool --ch combinations in the existing oiiotool-copy test). Rather, we aim to MOVE the text of the example out of the documentation file, and into a test, and then the docs merely reference it in the other file, so that it can never diverge.

Now, I think you may be the first to do this for the oiiotool chapter, and I fear that it exposes that I have omitted something you probably needed: While I have testsuite/docs-examples-cpp and testsuite/docs-examples-python, it looks like I never made testsuite/docs-examples-oiiotool that gives you a place to put the code or a full example recipe to follow.

I will try to set up a single example the right way, and then you can use that as a template. Will try that as soon as I can.

@linsen458-spec

Copy link
Copy Markdown
Author

Ah, thanks for taking the time to explain — that makes sense now. I read it as "recreate the examples as tests", which as you point out still leaves room for the two copies to drift apart. Making them literally the same lines is a much better idea.

Happy to redo this once you've got the docs-examples-oiiotool template in place. No rush on my end — I'll keep an eye out and rework the PR when it lands. And since I'm apparently the first one stumbling through an oiiotool chapter, I'll happily jot down whatever friction I run into, if that's useful for the wiki recipe.

@lgritz

lgritz commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

I have some fixes for the runtest.py that will make this MUCH easier. Will try to get a PR for it tonight. Thanks for understanding.

@lgritz

lgritz commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

#5465

I hope it's clear what the application of this change to runtest.py is to your PR here -- you can now easily express exactly the text that somebody would type on the command line (and what you want to appear verbatim in the docs) as part of the testsuite/oiiotool*/ tests, including the ability to mark specific sequences of lines to reproduce in the docs by reference.

If you or anybody reading would like to review my PR, that will let me merge it so that you can directly build off of it to complete this PR.

You may find that in order to complete the docs/tests synchronization, you may need to do one or more of (a) add tests that match the docs; (b) change the docs slightly so that its examples use what we already have in the tests; (c) reorder or slightly change what's in the tests to match the docs or to group things that need to be part of the same docs examples; (d) add new things to one or both or do other kinds of synchronization. Use your best judgment here. But there's no point in the tests-for-tests-sake and tests-for-docs to be rudundant unnecessarily; any parts that can do both simultaneously is better than doing the same thing twice (and making the CI tests more expensive) where one is just for the sake of the docs.

@lgritz

lgritz commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

I think what you did here is combine your changes with the commit that contains my recent runtest.py changes. Can you rebase this on the current main and then force-push? What I would really like to see, to make it easiest to review this PR, is for the diff to only contain your changes, and not some combination of yours, mine, and any changes you might have made on top of my changes.

…atim testsuite test

The oiiotool command lines from the 'Channel reordering and padding'
section of the oiiotool docs now live verbatim inside run_commands()
blocks in the run.py, bracketed by BEGIN/END-docs-channels-* marker
comments, and src/doc/oiiotool.md literalincludes each command between
its markers -- so the docs and the test are literally the same lines and
can no longer drift apart.

The setup images use asymmetric per-channel colors on purpose: with the
default black/white checker, R and B are identical in every pixel, which
made the channel-swap and alpha-from-red examples produce outputs
indistinguishable by content hash.

Contributes to AcademySoftwareFoundation#3992

Assisted-by: Claude Code / glm-5.3-flash
Signed-off-by: linsen <251731047+linsen458-spec@users.noreply.github.com>
@linsen458-spec
linsen458-spec force-pushed the test/doc-example-conversion branch from c0cb07b to fb454b6 Compare September 14, 2026 07:13
@linsen458-spec

Copy link
Copy Markdown
Author

You're right, sorry — that was my mistake. When rebuilding the branch, I accidentally amended my commit on top of yours, which swallowed your runtest.py changes into the PR diff. I've fixed it now. Thanks for your patience in reviewing and replying.

@lgritz

lgritz commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

This looks great. I would actually be fine with merging this as it is now, though I have some questions, and I'm not asking you to necessarily change anything (not yet, anyway), I just am interested in how you think we should proceed next.

You made a new test, oiiotool-channels, with oiiotool commands that are examples of --ch, simply moved from text in the documentation to executed commands in this test. It has only that, and is probably by far the smallest and fastest-running "oiiotool-*" test directory we have.

But we already did have tests for the --ch command in the oiiotool-copy test, along with closely related commands like --chappend and --chnames. The oiiotool-copy test has a theme, overall, of collecting all the tests that are related to copying and shuffling various things -- channels, subimages, pixel data format changes, etc. (Aside: At some point these were separated from their original birthplace in "testsuite/oiiotool", which was getting too big and took too long to run individually. There is a certain maximum size/runtime we want any individual testsuite to get.)

So how should we reconcile this? I think this question has two parts:

  • Location: Should we actually move these few new tests into oiiotool-copy with the other things that test --ch? Or should we use the excuse to copy all the other channel-related tests from oiiotool-copy into oiiotool-channels? Or just leave it as it is for now?
  • Functionality coverage: I'm betting that several of the examples written for the docs that are now in oiiotool-channels are functionally the same as existing tests in oiiotool-copy that were written to actually verify the software. Especially if we sought to put those two sets of tests in the same file, we may wish to eliminate the needless redundancy, either by having the docs just use the old tests as their examples, or use the docs versions as the real functionality tests, or some other sensible combination that helps eliminate the "duplicates" (which are not textual duplicates, that's the tricky part).

Am I rambling?

@linsen458-spec

Copy link
Copy Markdown
Author

Not rambling at all — if anything, it's good to see how much you care about getting this right. This is exactly the kind of project that makes me happy to spend the time.

So I went back through both run.py files and diffed the examples against what oiiotool-copy already has. Two things stood out.

On where they live: the two directories are doing different jobs. oiiotool-copy groups by feature, and a lot of its lines lean on files created earlier in the same run.py — src/rgbaz.exr only exists because of the lines above it. oiiotool-channels follows the docs instead, with each example self-contained and the setup commands at the top making whatever it needs.

That's mostly why I'd leave the examples where they are. A line lifted from the middle of oiiotool-copy's run.py would leave a reader wondering where the input came from. Moving all the channel tests out is also riskier than it's worth: some of it is edge-case testing (the PNG channel-count warning, for one) that reads oddly as a doc example, and I'd rather not churn a passing test for no clear win. One small directory per section also fits the size and runtime limits you mentioned better than growing the big ones.

Then the duplicates. I went through them, and the overlap is smaller than it looks. Of the seven examples, only one repeats what oiiotool-copy already covers (--ch R,G,B against the --ch R,G,B,A test over in oiiotool-copy). The other six hit things copy never touches: zeroing channels by name, swapping two channels, -i:ch at open time, and the three ways of adding alpha/Z.

For that one I'd keep both, honestly. Different inputs, different outputs, and it costs about nothing. If we want a rule going forward, the one you floated earlier works for me: before a docs example becomes a test, check whether an existing test already covers it. Reads fine on its own? Let the docs reuse that line. Depends on earlier setup? The example stays with its section and the old test keeps its job.

I'll apply that to the rest of #3992 unless you object. And if you'd rather consolidate by feature and fold these into oiiotool-copy, that's a quick change too — say the word.

@lgritz

lgritz commented Sep 19, 2026

Copy link
Copy Markdown
Collaborator

If the individual test lines we primarily use for full test coverage just aren't appropriate for what we want in the docs for clear elucidation, then let's keep them both and not worry about a little redundancy.

However, if you don't mind, I think I would prefer that these few new tests just land in oiiotool-copy, near the others that also exercise --ch and related commands, rather than making a separate test directory just for them.

Sometimes when an individual test directory is seen to be very big or expensive, we do try to split it (into roughly equal portions if possible, and hopefully with some obvious logic to which tests go in which directory). In this case, the new tests are few, and they sure seem logically connected to things in oiiotool-copy, so let's not make a new directory for this.

Sorry to ask you to do the extra work. I believe this is the last thing to change on this PR.

By the way, did you intend this as a DevDays project?

@lgritz lgritz added build / testing / port / CI Affecting the build system, tests, platform support, porting, or continuous integration. docs Documentation labels Sep 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build / testing / port / CI Affecting the build system, tests, platform support, porting, or continuous integration. docs Documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants