Skip to content

fix(python): make the assert_not_equal failure message describe the failure#4778

Open
dbrattli wants to merge 1 commit into
mainfrom
fix/py-assert-not-equal-message
Open

fix(python): make the assert_not_equal failure message describe the failure#4778
dbrattli wants to merge 1 commit into
mainfrom
fix/py-assert-not-equal-message

Conversation

@dbrattli

Copy link
Copy Markdown
Collaborator

Follow-up to #4776, where the same wart surfaced on the Beam target.

On a failing Assert.NotEqual, assert_not_equal renders:

Expected: 1 - Actual: 1

The two values being equal is precisely why the assertion failed, so the message describes a passing assertion. Staring at a red test, it tells you nothing — and it briefly reads as though the test framework is broken. It happens because assert_equal and assert_not_equal share one format string:

def assert_not_equal[T](actual: T, expected: T, msg: str | None = None) -> None:
    if equals(actual, expected):
        raise Exception(msg or f"Expected: {expected} - Actual: {actual}")   # same string as assert_equal

Now it says what was actually expected — some value other than this one:

Expected not equal to: 1 - Actual: 1

Prior art

xUnit, which these same tests run against on .NET before being transpiled, goes out of its way to distinguish the two cases:

Assert.Equal() Failure: Values differ          Assert.NotEqual() Failure: Values are equal
Expected: 1                                    Expected: Not 1
Actual:   2                                    Actual:       1

The Beam target already words it this way (#4775). This brings Python into line, keeping the Fable house format (Expected... - Actual...) rather than adopting xUnit's layout.

Scope: JS is deliberately left alone

fable-library-ts's Util.ts:assertNotEqual has the identical wart. I have not touched it — that's a call for the JS/TS maintainers, and I'd rather use this PR to ask than to unilaterally change a message on the most-used target.

@fable-compiler maintainers: do you want the same one-line change on JS/TS (and Dart/Rust, which don't currently implement Assert at all), so all targets agree? Happy to fold it in here or leave it for a separate PR — or to drop this one if you'd rather all targets keep matching each other, warts and all.

Compatibility

fable-library-py is on PyPI and must stay backward-compatible within a minor version. This changes no signature, no raising behaviour, and no return value — only the human-readable text of a message that, by construction, is produced only on an already-failing assertion. Nothing in the repo depends on the old wording. Flagging it explicitly since AGENTS.md asks for care with the semantics of existing fable-library-py functions.

Tests

New src/fable-library-py/tests/test_assert.py covers both functions: pass, fail, and custom-message paths. The not-equal message test fails on main and passes here; the rest pass either way.

Full fable-library-py suite: 444 passed. Pyright clean on both files.

🤖 Generated with Claude Code

…ailure

On a failing Assert.NotEqual, assert_not_equal rendered "Expected: 1 - Actual: 1"
-- the two values are equal, which is *why* it failed, so the message reads as a
passing assertion and tells you nothing. Say what was actually expected instead:
a value other than this one.

xUnit, which the same tests run against on .NET, distinguishes the two cases for
this reason ("Assert.NotEqual() Failure: Values are equal / Expected: Not 1").
The Beam target already words it this way; JS has the same wart but is left alone
here, pending a maintainer decision.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

Python Type Checking Results (Pyright)

Metric Value
Total errors 34
Files with errors 4
Excluded files 4
New errors ✅ No
Excluded files with errors (4 files)

These files have known type errors and are excluded from CI. Remove from pyrightconfig.ci.json as errors are fixed.

File Errors Status
temp/tests/Python/test_hash_set.py 18 Excluded
temp/tests/Python/test_applicative.py 12 Excluded
temp/tests/Python/test_nested_and_recursive_pattern.py 2 Excluded
temp/tests/Python/fable_modules/thoth_json_python/encode.py 2 Excluded

@dbrattli dbrattli requested a review from MangelMaxime July 14, 2026 14:22
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.

1 participant