Skip to content

fix homebrew a bit#1047

Merged
sbryngelson merged 5 commits intomasterfrom
homebrew-new
Nov 15, 2025
Merged

fix homebrew a bit#1047
sbryngelson merged 5 commits intomasterfrom
homebrew-new

Conversation

@sbryngelson
Copy link
Member

@sbryngelson sbryngelson commented Nov 15, 2025 •
edited by qodo-code-review bot
Loading

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-code-review
Copy link
Contributor

qodo-code-review bot commented Nov 15, 2025

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-code-review
Copy link
Contributor

qodo-code-review bot commented Nov 15, 2025 •
edited
Loading

PR Code Suggestions ✨

No code suggestions found for the PR.

Copilot AI reviewed Nov 15, 2025
View reviewed changes
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

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

Review effort 1/5

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@sbryngelson