Skip to content

Fix the grcbc outflow tests that #1854 broke - #1889

Merged
sbryngelson merged 1 commit into
MFlowCode:masterfrom
sbryngelson:fix/grcbc-outflow-tests
Sep 17, 2026
Merged

sbryngelson merged 1 commit into
MFlowCode:masterfrom
sbryngelson:fix/grcbc-outflow-tests

Conversation

@sbryngelson

Copy link
Copy Markdown
Member

Description

Master has failed Lint Toolchain / Toolchain Compatibility on every push and every open PR since 386e615 (#1854). This is what is behind the red lint gate on #1887, #1867, and anything else rebased onto current master — none of those PRs touch the toolchain.

The cause is a merge-order collision. #1877 added TestGrcbcOutflowTargets with bc_x%grcbc_in = "T" in its fixture but never supplied the inflow state. #1854, merged next, correctly began requiring that full state. Each PR was green against the master it branched from; together they fail:

Subsonic Inflow (grcbc_in) needs the full inflow state; missing bc_x%pres_in, bc_x%vel_in(1), ...

grcbc_in and grcbc_out are independent switches, and these four tests exercise only the outflow branch, so the fix is to leave grcbc_in off in the fixture. A -7 boundary without grcbc_in is a plain subsonic inflow, which is all the far side of the fixture needs. Neither validator changes — both #1854's and #1877's checks are correct as written.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Scope

  • This PR comprises a set of related changes with a common goal

How Has This Been Tested?

  • pytest mfc/test_case_validator.py: 73/73 pass (was 69/73). Bisected the three commits touching the validator since the tests were added: passes at 63c0c9e and 6265185, fails at 386e615.
  • ./mfc.sh precheck (full toolchain test suite): passes.

Checklist

  • I have added comments for the new code
  • I ran ./mfc.sh format before committing my code
  • New and existing tests pass locally with my changes, including with GPU capability enabled (both NVIDIA hardware with NVHPC compilers and AMD hardware with CRAY compilers) and disabled — n/a, toolchain-only change

Diagnosed and fixed with Claude Code.

MFlowCode#1877 added TestGrcbcOutflowTargets with grcbc_in = T in its fixture but never
supplied the inflow state. MFlowCode#1854, merged next, correctly began requiring that
state, so master has failed Lint Toolchain / Toolchain Compatibility on every
push and every open PR since 386e615.

grcbc_in and grcbc_out are independent switches, and these tests exercise only
the outflow branch, so the fix is to leave grcbc_in off. A -7 boundary without
grcbc_in is a plain subsonic inflow, which is all the far side of the fixture
needs. Neither validator changes.

Diagnosed and fixed with Claude Code.
Copilot AI lite review requested due to automatic review settings September 17, 2026 15:44
@sbryngelson
sbryngelson merged commit 665ba86 into MFlowCode:master Sep 17, 2026
20 checks passed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Pull request overview

Fixes the TestGrcbcOutflowTargets fixtures so they no longer enable grcbc_in without supplying the required inflow state, restoring validator and toolchain test compatibility.

Changes:

  • Removes bc_*%grcbc_in from the outflow test fixtures.
  • Adds comments documenting why grcbc_in remains disabled while grcbc_out is tested.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@codecov

codecov Bot commented Sep 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.53%. Comparing base (55e62be) to head (23c585e).
⚠️ Report is 11 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1889      +/-   ##
==========================================
+ Coverage   61.35%   61.53%   +0.17%     
==========================================
  Files          84       84              
  Lines       22224    22341     +117     
  Branches     3255     3271      +16     
==========================================
+ Hits        13636    13747     +111     
+ Misses       6152     6144       -8     
- Partials     2436     2450      +14     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants