Skip to content

Conversation

@sbryngelson
Copy link
Member

@sbryngelson sbryngelson commented Nov 15, 2025

User description

homebrew fixes


PR Type

Bug fix


Description

  • Remove "run" subcommand from mfc test invocation

  • Simplify test case execution in Homebrew formula


Diagram Walkthrough

flowchart LR
  A["mfc test command"] -- "remove 'run' subcommand" --> B["simplified invocation"]
Loading

File Walkthrough

Relevant files
Bug fix
mfc.rb
Remove run subcommand from mfc test                                           

packaging/homebrew/mfc.rb

  • Removed "run" subcommand from the mfc command invocation in the test
    block
  • Changed from system bin/"mfc", "run", "case.py", "-j", "1" to system
    bin/"mfc", "case.py", "-j", "1"
  • Simplifies the test execution by calling mfc directly with the case
    file
+1/-1     

Copilot AI review requested due to automatic review settings November 15, 2025 03:32
@cursor
Copy link

cursor bot commented Nov 15, 2025

You have run out of free Bugbot PR reviews for this billing cycle. This will reset on December 8.

To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

@qodo-merge-pro
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Possible Issue

Ensure that removing the 'run' subcommand still triggers both pre_process and the simulation when invoking 'mfc case.py -j 1'; if 'run' was required, the test may now be a no-op or only parse the case.

# Run the case from the test directory (this will execute pre_process and simulation)
# Limit to 1 processor and reduce runtime for testing
cd testpath_case do
  system bin/"mfc", "case.py", "-j", "1"
end
Flaky Test

Homebrew tests run in constrained CI; verify that 'mfc case.py -j 1' completes quickly and deterministically and still produces 'silo_hdf5' to avoid timeouts or intermittent failures.

# Run the case from the test directory (this will execute pre_process and simulation)
# Limit to 1 processor and reduce runtime for testing
cd testpath_case do
  system bin/"mfc", "case.py", "-j", "1"
end

# Verify output files were created in the test directory
assert_path_exists testpath_case/"silo_hdf5"

@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Nov 15, 2025

PR Code Suggestions ✨

No code suggestions found for the PR.

Copilot finished reviewing on behalf of sbryngelson November 15, 2025 03:34
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes the Homebrew formula test to use the correct command-line syntax for the MFC wrapper. The Homebrew wrapper uses a simplified interface where users run mfc <case.py> directly, without the "run" subcommand that's used in the full ./mfc.sh developer interface.

Key Change

  • Corrected the test invocation from mfc run case.py to mfc case.py, aligning with the Homebrew wrapper's documented behavior

@sbryngelson sbryngelson merged commit 9333c68 into master Nov 15, 2025
41 checks passed
@sbryngelson sbryngelson deleted the homebrew-new branch November 15, 2025 04:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants