Check out correct ref in "pytest" CI workflow

[khaeru]

#281 shifted the main event trigger for the "pytest" CI workflow from pull_request to pull_request_target. However, it erroneously omitted to explicitly specify the ref to be checked out. This is necessary because actions/checkout (and in this repo also francisbilham11/action-cached-lfs-checkout) will default to checking out $GITHUB_REF; for the pull_request_target event, this is the base branch commit, unlike for pull_request, where it is the source branch commit.

As a consequence of this, when merging #259, the "pytest" workflow appeared to pass, but this was in fact running the tests on main, while the tests did not all pass on the PR branch. Thus when #279 #282 were updated with rebase, they picked up these failing tests.

This PR:

• Corrects the pytest workflow, in a similar way to iiasa/message-ix-models#913.
• Fixes the tests that actually were failing on Transport improvements from 2024-W49 #259.

And also:

• Updates the copyright year to 2025.
• Drops support for ixmp/message_ix 3.4.0 and 3.5.0, in line with our stated Upstream version policy.
• Reduces the verbosity for some transport tests. The previous setting produced very verbose logs (~1.2 GiB) for some of the above failing runs.

How to review

• Note the CI checks all pass.
• After this, I will rebase the branch to drop the one "TEMPORARY" commit, and then merge.

PR checklist

• Continuous integration checks all ✅
• Add or expand tests; coverage checks both ✅
• Add, expand, or update documentation. N/A
• Update doc/whatsnew.

[codecov]

Codecov Report

Attention: Patch coverage is 90.90909% with 6 lines in your changes missing coverage. Please review.

Project coverage is 77.0%. Comparing base (08dc7fc) to head (9a130e9).

Files with missing lines	Patch %	Lines
message_ix_models/util/cache.py	75.0%	3 Missing ⚠️
message_ix_models/util/context.py	82.3%	3 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##            main    #288      +/-   ##
========================================
+ Coverage   65.7%   77.0%   +11.3%     
========================================
  Files        213     213              
  Lines      16463   16512      +49     
========================================
+ Hits       10817   12724    +1907     
+ Misses      5646    3788    -1858     

Files with missing lines	Coverage Δ
message_ix_models/model/transport/config.py	97.3% <100.0%> (+5.3%)	⬆️
message_ix_models/model/transport/ldv.py	96.7% <100.0%> (+75.8%)	⬆️
message_ix_models/model/transport/testing.py	100.0% <100.0%> (+24.2%)	⬆️
message_ix_models/project/ssp/cli.py	94.7% <100.0%> (+47.3%)	⬆️
message_ix_models/report/__init__.py	95.6% <100.0%> (+5.8%)	⬆️
message_ix_models/testing/__init__.py	76.7% <100.0%> (-17.1%)	⬇️
...sage_ix_models/tests/model/transport/test_build.py	77.9% <ø> (+13.2%)	⬆️
...ssage_ix_models/tests/model/transport/test_data.py	85.5% <100.0%> (+38.1%)	⬆️
...age_ix_models/tests/model/transport/test_ikarus.py	100.0% <ø> (+60.0%)	⬆️
...essage_ix_models/tests/model/transport/test_ldv.py	97.5% <ø> (+61.7%)	⬆️
... and 8 more

... and 40 files with indirect coverage changes

[glatterf42]

This looks good to me except for the failing tests. Are these errors just flaky? I'll try rerunning them.
Other than that, I'm just wondering if test times are getting somewhat out of hand and if there's anything we can do about that. For v3.8.0, the macos test failed after 43 minutes.

[khaeru]

This looks good to me except for the failing tests. Are these errors just flaky? I'll try rerunning them.

Seems not, but I'm baffled why they'd fail on macOS and not on others. I'll investigate.

[khaeru]

I'm baffled why they'd fail on macOS and not on others. I'll investigate.

Okay, something very strange is happening on macOS. Comparing this run (failing) to this run (passing), I see:

message_ix_models.tests.model.transport.test_report.test_plot_simulated: test_plot_simulated: test_context.regions = 'R12'
message_ix_models.model.transport.testing.simulated_solution: simulated_solution: context.regions = 'R12'
message_ix_models.model.transport.testing.built_transport: built_transport: context.regions = 'R12'
message_ix_models.testing.bare_res: bare_res: context.model.regions = 'R12'
message_ix_models.testing.bare_res: bare_res: base.set('node') = 0        World
1      R12_AFR
…
12     R12_WEU
dtype: object
message_ix_models.testing.bare_res: Clone to 'MESSAGEix-GLOBIOM R12 YB 41eee/test_plot_simulated[stock-ldv]'
message_ix_models.model.transport.testing.built_transport: built_transport: res.set('node') = 0        World
1      R12_AFR
…
12     R12_WEU
dtype: object
message_ix_models.model.transport.config.from_context: .transport.Config.from_context: context.model.regions = 'R12'
message_ix_models.util.common._load: 'transport set.yaml' already loaded; skip
message_ix_models.util.common._load: 'transport technology.yaml' already loaded; skip
message_ix_models.model.transport.testing.built_transport: Clone to 'MESSAGEix-Transport R12 YB 41eee/test_plot_simulated[stock-ldv]'
message_ix_models.model.transport.testing.simulated_solution: simulated_solution: scenario.set('node') = 0       World
1     R11_AFR
…
11    R11_WEU
dtype: object
message_ix.report.from_scenario: Scenario "MESSAGEix-Transport R12 YB 41eee/test_plot_simulated[stock-ldv]" has no solution
message_ix.report.from_scenario: Some reporting may not function as expected

versus

message_ix_models.tests.model.transport.test_report.test_plot_simulated: test_plot_simulated: test_context.regions = 'R12'
message_ix_models.model.transport.testing.simulated_solution: simulated_solution: context.regions = 'R12'
message_ix_models.model.transport.testing.built_transport: built_transport: context.regions = 'R12'
message_ix_models.testing.bare_res: bare_res: context.model.regions = 'R12'
message_ix_models.testing.bare_res: bare_res: base.set('node') = 0        World
1      R12_AFR
…
12     R12_WEU
dtype: object
...bare_res  bare_res: base.set('node') = 0        World
1      R12_AFR
…
12     R12_WEU
dtype: object
message_ix_models.testing.bare_res: Clone to 'MESSAGEix-GLOBIOM R12 YB 41eee/test_plot_simulated[stock-ldv]'
message_ix_models.model.transport.testing.built_transport: built_transport: res.set('node') = 0        World
1      R12_AFR
…
12     R12_WEU
dtype: object
message_ix_models.model.transport.config.from_context: .transport.Config.from_context: context.model.regions = 'R12'
message_ix_models.util.common._load: 'transport set.yaml' already loaded; skip
message_ix_models.util.common._load: 'transport technology.yaml' already loaded; skip
message_ix_models.model.transport.testing.built_transport: Clone to 'MESSAGEix-Transport R12 YB 41eee/test_plot_simulated[stock-ldv]'
message_ix_models.model.transport.testing.simulated_solution: simulated_solution: scenario.set('node') = 0        World
1      R12_AFR
…
12     R12_WEU
dtype: object
message_ix.report.from_scenario  Scenario "MESSAGEix-Transport R12 YB 41eee/test_plot_simulated[stock-ldv]" has no solution
message_ix.report.from_scenario: Some reporting may not function as expected

In other words, this clone():

message-ix-models/message_ix_models/model/transport/testing.py

Line 141 in 08dc7fc

return scenario.clone(scenario=request.node.name, keep_solution=solved)

…somehow, on macOS, returns a different scenario with R11 nodes instead of R12 🤔

[khaeru]

As mentioned on Slack, I've put the following in .transport.testing.built_transport():

    if (
        GHA
        and platform.system() == "Darwin"
        and identify_nodes(result) != context.model.regions
    ):
        pytest.xfail(
            reason="Known issue on GitHub Actions macOS runners: result has nodes "
            f"{identify_nodes(result) = !r} != {identify_nodes(res) = !r} == "
            f"{context.model.regions = !r}"
        )

…until time is available to get to the bottom of the cause.

I have a vague suspicion it may be due to using pytest-xdist, but since I don't use built_transport() in production, on macOS, or using parallel processes via Popen; and since the same tests still pass on Linux and Windows, I feel this is an acceptable workaround.

[glatterf42]

Thanks for this cleanup :)

[khaeru]

Thanks! So with the approval and this run in which all checks pass, I will rebase to drop the "TEMPORARY" commit and then immediately merge.

[khaeru]

To confirm all is right after the merge, I've rebased #279 which starts this workflow run and will monitor to ensure that the jobs all succeed.

[khaeru]

will monitor to ensure that the jobs all succeed.

✅