Skip to content

Fix CI#413

Merged
LPTK merged 5 commits intohkust-taco:hkmc2from
AnsonYeung:fix-handler-error
Mar 14, 2026
Merged

Fix CI#413
LPTK merged 5 commits intohkust-taco:hkmc2from
AnsonYeung:fix-handler-error

Conversation

@AnsonYeung
Copy link
Contributor

This fixes the issue of the non deterministic test failure.

//│ ═══[RUNTIME ERROR] RangeError: Maximum call stack size exceeded


// * For some reason, this works locally with values 100 and 100,
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice to still leave a comment explaining what was happening here before this change (the cause of the flakiness).

@LPTK LPTK mentioned this pull request Mar 13, 2026
Copy link
Contributor

@LPTK LPTK left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM.

Comment on lines +276 to +277
// See https://github.com/hkust-taco/mlscript/pull/410
// This test was failing non-deterministically due to handler state not being properly reset.
Copy link
Contributor

Choose a reason for hiding this comment

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

Come on. That's a very poor effort at documentation. That PR does nor povide any explanation.

Suggested change
// See https://github.com/hkust-taco/mlscript/pull/410
// This test was failing non-deterministically due to handler state not being properly reset.
// This test was failing non-deterministically due to handler state not being properly reset.
// The flakiness was due to the stack overflow in the previous block not always interrupting the code in the same place,
// sometimes leaving the effect handlers runtime in an inconsistent state.

@LPTK LPTK merged commit ee7c416 into hkust-taco:hkmc2 Mar 14, 2026
1 check passed
@LPTK LPTK deleted the fix-handler-error branch March 14, 2026 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants