Skip to content

Fix highspy test_user_interrupts - #3266

Open
galabovaa wants to merge 1 commit into
latestfrom
issue-3265
Open

galabovaa wants to merge 1 commit into
latestfrom
issue-3265

Conversation

@galabovaa

@galabovaa galabovaa commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Description

joinSolve's wait loops (while not result[0]: result = self.wait(0.1)) assume self.wait() always returns a (bool, HighsStatus | None) tuple. test_user_interrupts replaces wait with a lambda that raises SIGINT to simulate Ctrl+C during a blocking wait, expecting the resulting KeyboardInterrupt to propagate out of that call and get caught by the surrounding except KeyboardInterrupt. On some platforms/timings the interrupt doesn't propagate synchronously, so the lambda returns None instead, and the next loop check crashes with TypeError: 'NoneType' object is not subscriptable — matching the trace in #3265.

Guards both wait loops in joinSolve against a None result (result is None or not result[0]) so this keeps polling instead of crashing. Production wait() never returns anything but a tuple, so this only affects the race.

Checklist

  • I have read the contributing guidelines
  • This PR targets the latest branch
  • Tests are passing
  • Documentation was updated where relevant
  • This PR is not primarily AI-generated (per the AI contributions policy in CONTRIBUTING.md)

Related issue

Closes #3265

@codecov

codecov Bot commented Sep 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.29%. Comparing base (a2b94fa) to head (617e24b).
⚠️ Report is 36 commits behind head on latest.

Additional details and impacted files
@@           Coverage Diff           @@
##           latest    #3266   +/-   ##
=======================================
  Coverage   73.29%   73.29%           
=======================================
  Files         446      446           
  Lines      108244   108250    +6     
  Branches    17352    17345    -7     
=======================================
+ Hits        79342    79347    +5     
- Misses      28626    28627    +1     
  Partials      276      276           

☔ 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.

@mathgeekcoder

Copy link
Copy Markdown
Collaborator

I think there's a better way to fix this issue.

Technically result should never be None. It's only being set to that because of the unit test overriding the wait function AND the signal not raising a KeyboardInterrupt.

The following should be more reliable than signal.raise_signal(signal.SIGINT):

highspy.highs.Highs.wait = lambda self, t: raise KeyboardInterrupt("Simulated user interrupt")  # type: ignore[assignment]

BTW: Overriding the wait function is a manual and lazy mock; it might be better practice to do the following:

    # 1. Use a context manager to patch the method safely
    with patch.object(highspy.highs.Highs, 'wait') as mock_wait:
        # 2. Tell the mock to trigger the signal when called
        mock_wait.side_effect = lambda t: raise KeyboardInterrupt("Simulated user interrupt") 

        h.joinSolve(h.startSolve(), 0)
        h.startSolve()
        h.joinSolve(None, 0)

        with self.assertRaises(SystemExit):
            h.startSolve()
            h.joinSolve(None, 5)
            unittest.main(exit=False)

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants