Repository navigation
Add a test suite and migrate CI to testitem-workflow v2 - #37
Merged
Merged
Conversation
The package shipped no tests at all: no test/ directory, no Test entry in Project.toml, and the jlpkgbutler CI had nothing to run. Adds four test items: - API: get_model across all six AR6 SSP scenarios, checking the time dimension, that temperature is finite, that 2100 warming relative to the 1850-1900 mean is ordered by scenario forcing, and that it lands in a plausible range. - Truncated time horizon: running to 2100 reproduces the first 351 years of the run to 2300 exactly. - Original: get_model_original across the four RCP scenarios, with the same shape and ordering checks. - Monte Carlo Simulation: run_mcs writes the expected result files for a three trial run, and rejects a single trial run. Replaces the six jlpkgbutler workflows and .jlpkgbutler.toml with the shared testitem-workflow v2 plus a dependabot config, and raises the Julia compat lower bound from 1.6 to 1.10. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The new Monte Carlo test item fails on the 32-bit Windows CI legs. On a 32-bit platform an integer literal is an Int32, so `run_mcs(trials=3)` never reached the function body at all: the `trials::Int64` annotation rejected it with a TypeError. The Monte Carlo API was simply unusable there. Widening the annotation to Integer is not enough on its own, because Mimi's run method for a SimulationDef also takes an Int64, so the conversion happens at that call. The test item now exercises Int32 and Int64 explicitly so the 64-bit legs cover the 32-bit path too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Mimi's run method for a SimulationDef takes an Int, which is the platform's native width, so the previous Int64 conversion moved the 32-bit Windows failure from run_mcs's own signature into that call instead of fixing it. Int is Int32 on a 32-bit platform and Int64 on a 64-bit one, so converting to Int works on both. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of an effort to get every package holding MimiGIVE components onto the test-item framework.
This package is one of two in that set that shipped no tests at all — no
test/directory, noTestentry inProject.toml, and a jlpkgbutler CI that therefore had nothing to run. So this is less a migration than writing the suite that was missing, directly in test-item form.Tests
Four test items:
APIget_modelacross all six AR6 SSP scenarios: time dimension is 1750:2300, temperature is finite and the right length, 2100 warming relative to the 1850–1900 mean is ordered by scenario forcing (ssp119 < ssp126 < ssp245 < ssp460 < ssp370 < ssp585), and every scenario lands in 1.0–6.0 °CTruncated time horizonOriginalget_model_originalacross the four RCP scenarios, at its default horizon and at a common 1765–2300 one, with the same shape/finiteness checks and RCP26 < RCP45 < RCP60 < RCP85 orderingMonte Carlo Simulationrun_mcswith three trials into a tempdir writestrials.csvand both result files, the saved temperatures are finite and cover trials 1–3, and a single-trial run is rejectedEvery threshold and ordering was checked against an actual run of the model first rather than guessed. The two entry points are deliberately not compared against each other:
get_modelis AR6/SSP-driven andget_model_originalis RCP-driven, so they are not expected to agree numerically.CI
Replaces the six
jlpkgbutler-*workflows and.jlpkgbutler.tomlwith.github/workflows/juliaci.ymlcallingjulia-testitems/testitem-workflow@v2, plus a dependabot config for julia and github-actions.Compat
julialower bound goes from 1.6 to 1.10, matching MimiSSPs and MimiGIVE. Without this the workflow'sinclude-smallest-compatible-minor-versionsdefault spends a matrix leg on Julia 1.6. This is release relevant — a new release will be needed before MimiGIVE picks it up. Because this drops support for a released Julia version, the-DEVversion inProject.tomlis moved to1.1.0-DEVso the next release is a minor bump rather than a patch.Verification
All four test items pass locally via
julia_run_testitems(4/4 passed, ~78 s).🤖 This PR was opened by Claude (Claude Code).
A bug the new tests found
The first CI run was red on both 32-bit Windows legs, and it turned out to be a real defect rather than a bad test.
run_mcswas declaredrun_mcs(;trials::Int64 = 2237, ...). On a 32-bit platform an integer literal is anInt32, sorun_mcs(trials = 3)never reached the function body at all:The Monte Carlo API was simply unusable on 32-bit Julia. Widening the annotation to
Integeris not sufficient on its own, because Mimi'srunmethod for aSimulationDefalso takes anInt64— so the conversion now happens at that call instead. Second commit in this PR.The test item exercises
Int32andInt64explicitly, so the 64-bit legs cover the 32-bit path too rather than relying on the x86 runners to catch a regression.