zppy's overall testing strategy

[forsyth2]

@chengzhuzhang @tomvothecoder (cc @zhangshixuan1987, @xylar) This post expands on the discussion from today's meeting. It is a follow-up to #848, which was focused on pcmdi_diags, and two un-merged PRs (#849, which enables testing using frozen environments and #852, which enables partial updates of expected results)

The current test workflow

The current test workflow is documented here. With the merging of #774, many of those steps have been automated away; the mostly automated workflow is documented here. With either method, there are four possible cases:

1. All the tests pass
2. Some tests fail, and they are legitimate failures. The developers of the respective package will need to update it so that it passes the tests. That is, the expected results should not be updated because we really should not expect these new results!
3. Some test fail, but these are expected based on recent code changes. That is, the expected results should be updated to reflect the latest behavior.
4. Some tests fail, but because of dependency changes. A very common failure is that a dependency version change has changed the look of something. Perhaps all the pixels have been shifted/translated right, so the plot still looks correct, but is technically different. That is, the expected results should be updated to reflect the latest changes from dependencies.

In cases 3 & 4, we need to update the expected results. The process to do that is documented here. It includes running several updater scripts, depending on what test you're updating results for.

I've outlined two proposed changes below, corresponding to the two un-merged PRs mentioned above.

Proposed changes

Testing with frozen environments

In #848, @zhangshixuan1987 suggested we test using a frozen environment, so we can be sure it's changes we made that are causing the diffs. This would essentially eliminate case 4 above. With that in mind, I started implementing #849. However, @xylar pointed out that this makes the E3SM Unified release extremely difficult. We agreed here that the ideal solution would be:

• Do test run 1. This will use frozen dev environments to see if our packages have caused diffs.
• If the diffs are expected based on recent changes, we update the expected results. If they are not expected, we debug and return to step 1.
• Do test run 2. This time, we use cutting-edge dev environments. Any diffs must be because of dependency drift since by this point, we've updated the expected results based on our changes. Here, we would basically always update the expected results regardless of the diffs, unless we see something truly out of the ordinary, because these diffs are outside our control.
• Repeat again the following week, after more of our code changes have merged.

That is, test run 1 lets us know if we're in case 2 or 3 (we caused a failure, either expected or not), test run 2 lets us know if we're in case 4 (dependencies caused a failure). This is obviously a bit more of a hassle, but with the test script now automated, it should be more straight-forward.

Potential concerns:

• Adding environment freezing to the script proved trickier than could be done in one day, which is why Test with frozen envs #849 is still incomplete.
• Increased compute node usage from running the test more often.

Partially updating expected results

#852 updates the expected results-updater scripts mentioned above, such that only a portion of the expected results are updated. This is extremely useful in cases where we know we can update the expected results for some tasks, but not others. For example, on the 8/12 test, we knew we could update the expected results for e3sm_diags, global_time_series, and pcmdi_diags, but that we should not update the expected results yet for mpas_analysis.

Potential concerns:

• A partial update means that the expected results for one task might have been produced using an older dev environment than that used for a different task. The different tasks of course have different dev environments anyway, but the date/time stamps won't be correct now. For example, the ls -lt /lcrc/group/e3sm/public_html/zppy_test_resources (the expected results dir) now shows everything with a date stamp of 8/14, even though mpas_analysis wasn't updated yet.
  • Note 1: This can be resolved by diving deeper: ls -lt /lcrc/group/e3sm/public_html/zppy_test_resources/expected_comprehensive_v3 in fact shows a 8/14 update for e3sm_diags, global_time_series, and pcmdi_diags, and a 5/20 update for mpas_analysis, livvkit, and ilamb.
  • Note 2: livvkit and ilamb weren't updated either -- they had no failures, so there's not much point in re-writing their expected results.
  • Note 3: Previous expected results exist in /lcrc/group/e3sm/public_html/zppy_test_resources_previous/, so we can roll back to previous expected results if need be.

@tomvothecoder also had a suggestion that involved keeping track of the dependency versions. This is not currently done for the expected results directory. It is also worth pointing out we'd need a full dev environment printout not only for zppy but all the tasks it calls (e3sm_diags, mpas_analysis, etc.).

Why care about updating the expected results often?

Updating the expected results frequently is really important to ensuring we know what actual changes are being introduced. As noted above, a frequent occurrence is that a pixel shift produces a bunch of diffs even though the plots are actually 100% acceptable. Let's call that task1. Say we can't update the expected results though, because a different task, task2 has real failures to debug. The following week we merge a new feature on task1, but remember we still haven't updated the baseline expected results. Now, it's impossible for me to tell if all these diffs are just the pixel shift we already knew about or new bugs introduced by this new feature! Now imagine this goes on for a few weeks, perhaps with task3 or task4 failing as well. Now, the expected results are weeks or even months out of date. They've essentially become useless as a comparison point. (In fact, currently, the last expected results are from May 20, almost 3 months ago!)

[chengzhuzhang]

@forsyth2 thanks for organizing this discussion. I think two things would enhance the current framework, let me know if this makes sense, I’m sure there may be cases this hasn’t considered/covered.

1. A two-run setup to separate the two causes of diffs. It's a variation on Test with frozen envs #849's idea, but the goal is the same: run the tests two ways on the same code — once in the environment that produced the baseline (isolates our code's changes) and once in a fresh, up-to-date environment (the difference exposes dependency changes).
2. More automation + a fixed schedule. The image tests (test_images.py) still run manually, automating those will be helpful. Pairing that with a scheduler (e.g., a monthly scrontab/cron run) would give us a regular, hands-off cadence.

The problem

When the weekly test shows image diffs, we can't tell whether our code caused them or a dependency update did

Root cause

There are two independent sources of image diffs:

• Code drift — our changes to zppy / e3sm_diags / etc.
• Dependency drift — new releases of matplotlib, numpy, and the diag packages.

Today they land in a single test run, so neither can be read cleanly.

Current vs. proposed

	Current	Proposed
Test runs	One run	Two runs on the same code
Environment	Fresh install each time (deps always drifting)	Run A pinned to the baseline's env; Run B fresh
What a diff means	Code + deps, mixed — unclear	A vs baseline = code; B − A = deps
Baseline env	Not recorded	A lock file stored next to the expected results
Baseline freshness	Whatever was last blessed (months old)	Advances deliberately at each bless
Blessing	Update everything at once	Choose: advance code-only (A) or code+deps (B)
Image tests	Manual (test_images.py run by hand on a compute node)	Automated as part of the scheduled run

How it works

One baseline = a set of expected images + a lock file recording the exact resolved env that produced them (conda list --explicit, not dev.yml — the yaml re-resolves and drifts).

Each cycle runs the current code two ways:

• Run A (pinned): env rebuilt from the baseline lock → any diff = our code.
• Run B (drift): fresh env with the latest deps → B − A = dependencies.

A human reviews both, then makes one bless decision:

• Code + dep changes both fine → advance to B: new images, and re-snapshot the lock (this cycle's deps become next cycle's baseline).
• Code fine but a dep looks wrong → advance to A: keep the lock where it is, don't accept the bad dep yet.
• Code looks buggy → don't bless; fix it first.

The expected images and the lock always move together — that pairing is what keeps "a diff = our code" true.

Diffs outside our control (external package problems)

• Cosmetic (pixel / font / colormap shift, plot still correct) → bless + a one-line note. Routine.
• A real defect in a dependency (wrong / missing data) → don't bless a bug into the baseline. Instead hold the lock at the last-good version (that's what it's for), file upstream, and advance once fixed. If we must move deps anyway, xfail the affected test with a link to the upstream issue so it re-flags itself when fixed. Also tell the E3SM-Unified maintainers — users will hit it too.

What we're not doing (keeping it simple)

• No predicting the next E3SM-Unified. We pin to the env that made the current baseline, which is known. (This is Test with frozen envs #849's easier "Scenario 1"; we drop the "match Unified" Scenario 2.)
• No per-task bless tooling (Enable partial updates of expected results for testing #852). Running the two lanes already separates code from deps; picking A vs B covers the messy cases without new config surface.

Automation

The heavy tests already run end-to-end via tests/main_branch_testing/run_integration_test.bash (AUTO_MODE=true), except image comparison, which is still manual. Plan:

1. Wrap test_images.py in an srun/salloc step so it runs automatically.
2. Schedule the whole thing bi-weekly on the machine (scrontab on Perlmutter, cron on Chrysalis) to run both lanes.
3. Post the diffs as a report (e.g. auto-updating issue).

Blessing stays manual — automation reports, a human decides.

Relationship to the open PRs

This is essentially #849's two-run idea, made reproducible: the "frozen env" becomes a resolved lock file tied to each baseline that advances on every bless (rather than a re-created dev.yml, which drifts). It sets aside #852 — the A-vs-B choice handles selective acceptance without per-task selectors.

[forsyth2]

@chengzhuzhang Thanks for putting this together.

image tests (test_images.py) still run manually, automating those will be helpful.
The heavy tests already run end-to-end [...] except image comparison, which is still manual

Yes, that's true. They run on a compute node whereas the test script is run on a login node, so I was just running that final line on a compute node to avoid further complicating the script.

But yes, we can probably launch a SLURM job on a compute node from the script. We'd need to either add a wait to the script so it doesn't exit early or else just remember to wait until the SLURM job finishes before we check results. For a completely automated system, we'd need to add the wait, similar to what's done to wait on the jobs that zppy launches.

Pairing that with a scheduler (e.g., a monthly scrontab/cron run) would give us a regular, hands-off cadence.

Xylar set up a GitHub Actions cron job for pre-commit in .github/workflows/pre_commit_update_workflow.yml on E3SM-Project/zppy-interfaces#57, but I still need to figure out how to get something like that started on Chrysalis itself. (And if that works, maybe we can extend to Perlmutter and Compy so we can start updating their expected results more often than just the Unified testing periods).

a set of expected images + a lock file recording the exact resolved env that produced them

Ok, so I'm thinking we'd have a lock file in the expected results dir. It already contains lists of images to expect (image_list_expected_*.txt), so adding a lock file as another piece of metadata would work. That would serve to document the environments that created the particular expected results. Then, the lock file would stay with its expected results as we move them to the archive (to replace them with newer expected results).

Important: there is not just one lock file (or if there is just one, it's a concatenation of many environments). It's not just zppy that needs a dev environment; it's many of the called packages as well: e3sm_to_cmip, e3sm_diags, mpas_analysis, zppy-interfaces. That's 5 lock files in total then.

Guide to the expected results dir

Let's look at the curent expected results dir. ls /lcrc/group/e3sm/public_html/zppy_test_resources gives:

expected_bash_files                       image_list_expected_legacy_3.0.0_bundles.txt
expected_bundles                          image_list_expected_legacy_3.0.0_comprehensive_v2.txt
expected_comprehensive_v2                 image_list_expected_legacy_3.0.0_comprehensive_v3.txt
expected_comprehensive_v3                 image_list_expected_legacy_3.1.0_bundles.txt
expected_legacy_3.0.0_bundles             image_list_expected_legacy_3.1.0_comprehensive_v2.txt
expected_legacy_3.0.0_comprehensive_v2    image_list_expected_legacy_3.1.0_comprehensive_v3.txt
expected_legacy_3.0.0_comprehensive_v3    test_campaign_cryosphere_expected_files
expected_legacy_3.1.0_bundles             test_campaign_cryosphere_override_expected_files
expected_legacy_3.1.0_comprehensive_v2    test_campaign_high_res_v1_expected_files
expected_legacy_3.1.0_comprehensive_v3    test_campaign_none_expected_files
image_list_expected_bundles.txt           test_campaign_water_cycle_expected_files
image_list_expected_comprehensive_v2.txt  test_campaign_water_cycle_override_expected_files
image_list_expected_comprehensive_v3.txt  test_defaults_expected_files

These expected results are for integration tests that can be run even without the long zppy runs.

# For: pytest tests/integration/test_bash_generation.py
expected_bash_files

# For: pytest tests/integration/test_campaign.py
test_campaign_* _expected_files # 6 of these

# For: pytest tests/integration/test_defaults.py
test_defaults_expected_files

These expected results are for integration tests that rely on the long zppy runs

# For: pytest tests/integration/test_bundles.py
# From that file: "The bundles tests in this file use the same output as the bundles image tests!"
expected_bundles

# For: pytest tests/integration/test_images.py
expected_bundles
expected_comprehensive_* # 2 of these
expected_legacy_* # 6 of these
image_list_expected_* # 9 of these

The image_list_expected_* files list the images that are expected. In that sense, they're more metadata than actually expected results. The images themselves are stored under the expected_* subdirectories. We could add the lock file(s) at this level as well, as another piece of metadata about the expected results.

Then, as we update expected results, the lock file(s) will still live with its associated expected results. (We'd move /lcrc/group/e3sm/public_html/zppy_test_resources/ to /lcrc/group/e3sm/public_html/zppy_test_resources_previous/expected_results_until_yyyymmdd).

Code + dep changes both fine → advance to B: new images, and re-snapshot the lock (this cycle's deps become next cycle's baseline).
Code fine but a dep looks wrong → advance to A: keep the lock where it is, don't accept the bad dep yet.
Code looks buggy → don't bless; fix it first.

We could do the two test runs with different UNIQUE_IDs: unique_id_frozen_env & unique_id_new_env.

So the workflow would be:

• We review diffs between unique_id_frozen_env and the baseline expected results:
  • They look buggy -> That means the code we added is buggy. This test run has no further actions. We need to fix the code and see how the next code goes. (Case 3 above)
  • They look to be desired based on recent code changes -> we now review diffs between unique_id_new_env and unique_id_frozen_env:
    • They look buggy -> That means the dependencies are buggy. We now update the expected results to match unique_id_frozen_env. (Case 2 above; note this is a rare case -- usually we'll just have to accept whatever the dependencies change).
    • They look acceptable (e.g., pixel shift) -> we now update the expected results to match unique_id_new_env AND we update the lock files. (Case 1 above)

This is almost functionally equivalent to the solution noted in #849. It's just that rather than updating the expected results twice, we do the comparison in our own workspace before pushing the one we want to the "official" expected results dir. That seems reasonable.

It seems like the other big change from that PR is noted later as "resolved lock file tied to each baseline that advances on every bless (rather than a re-created dev.yml, which drifts)".

No predicting the next E3SM-Unified. We pin to the env that made the current baseline, which is known.

I think the AI has gotten confused here. Nothing in #849 suggested we predict what versions will used in the next E3SM-Unified.

I think it's referring to these bullets:

If the current "expected results" were generated using dev environments, then we just need to freeze the versions used there.
If the current "expected results" were generated using the Unified release, then we need to freeze a dev environment that matches up with Unified as much as possible.

That last bullet point is trying to say that on the very first test run we do after a Unified release, the "expected results" will be coming from a Unified env, not a dev environment. Those are set up differently, which may complicated the process.

That is, the point was about matching the previous Unified, not the next one.

Post the diffs as a report

Yes, this is one thing #774 didn't do. I'm still manually putting together the Markdown reports on #845. Ideally, we'd update the script to write the Markdown report too. Then the review/bless stage would be extremely simple from the human perspective.

It sets aside #852 — the A-vs-B choice handles selective acceptance without per-task selectors.

I don't mind making that a separate PR to reduce scope, but it should still be addressed.

Take the current situation we're in. I was able to update results for e3sm_diags, mpas_analysis, and global_time_series instead of waiting for mpas_analysis to also be marked as ready to update. All our packages are in development -- indeed that's why we test with the dev environments.

Say e3sm_diags has diffs that need debugging in week 1, so we can't update expected results for anything. Then in week 2, e3sm_diags has fixed the bug but now there's a bug in mpas_analysis, so we still can't update expected results. The process could continue ad infintium.

[tomvothecoder]

Xylar set up a GitHub Actions cron job for pre-commit in .github/workflows/pre_commit_update_workflow.yml on E3SM-Project/zppy-interfaces#57, but I still need to figure out how to get something like that started on Chrysalis itself. (And if that works, maybe we can extend to Perlmutter and Compy so we can start updating their expected results more often than just the Unified testing periods).

@TonyB9000 might be able to inform you here. He was able to get a cron job working with SimBoard to run periodic metadata scanning of data directory on LCRC. The same design principle can be applied to run tests periodically.

This cron job can set up on your user account (at least on LCRC/Chrysalis). You'd need to verify on Perlmutter and Compy.

[TonyB9000]

@tomvothecoder @forsyth2 @chengzhuzhang On every Linux system I know, a user is given the rights to a personal "crontab" file, operated by root but limited in permissions to the ordinary user account permissions.

    crontab -e        [to edit]
    crontab -l        [to list what's there]

If a user (someone with an account) can execute a bash script that loops, sleeps and runs other stuff, then there is no added privilege in having a crontab as well. The added benefits of a crontab are:

1. Ease of scheduling
2. Continued operation is automatic following reboots.

Where one has very complex, multi-application workflows with operational inter-dependencies, a workflow orchestrator like "Jenkins Pipelines" may be of service, but I have personally never needed such involved scheduling controls.

At the very least, simple crontab entries can launch control scripts that assess other dependencies for prototyping more complex flows.

[forsyth2]

@xylar noted on the 8/12 test results thread:

This shows them really doing their job in two respects: 1) catching a regression that would come to bite us in the next unified and; 2) showing a new version of a dependency (in this case matplotlib) causing us trouble so we have enough time to fix it.

This is also an example of why I think testing with the latest dependencies is the most important thing the testing does, even if it creates extra pain and confusion. This wouldn't have been caught if we tested with fixed dependencies.

This makes me think #852 (allow partial updates of expected results) is the higher priority issue compared with #849 (double-test: once with our changes, once with dependency changes). Xylar's note here reveals a flaw in my line of thinking noted in the "Proposed changes" section above. Specifically, I had said:

Here, we would basically always update the expected results regardless of the diffs, unless we see something truly out of the ordinary, because these diffs are outside our control.

But "we see something truly out of the ordinary" isn't actually that rare of a case. Dependency version updates can and do reveal/create bugs in our code. As noted in the same thread, some diffs were purely cosmetic but others were genuine bugs.

My takeaway here is that testing against a frozen env is less valuable than I had originally thought. I think it's more important that we're able to update the expected results we know are correct, so that they don't get "stale" waiting for everything to be perfect.

@chengzhuzhang with that in mind, I'm thinking we should direct our attention to merging #852, but with some sort of printout/log of what environment was used for each task (not just zppy but also e3sm_diags, mpas_analysis, etc.), so that it's better documented. What do you think?

[chengzhuzhang]

@chengzhuzhang with that in mind, I'm thinking we should direct our attention to merging #852, but with some sort of printout/log of what environment was used for each task (not just zppy but also e3sm_diags, mpas_analysis, etc.), so that it's better documented. What do you think?

Yeah, that sounds fine. PR merges are not very frequent for these tools. Manually triggered testing should be sufficient for now.