-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[stdlib] assertion overloads for List
#3372
Conversation
dabe398
to
b2f8597
Compare
def test_assert_messages(): | ||
try: | ||
assert_true(False) | ||
except e: | ||
assert_true("test_assertion.mojo:33:20: AssertionError:" in str(e)) | ||
|
||
try: | ||
assert_false(True) | ||
except e: | ||
assert_true("test_assertion.mojo:38:21: AssertionError:" in str(e)) | ||
|
||
try: | ||
assert_equal(1, 0) | ||
except e: | ||
assert_true("test_assertion.mojo:43:21: AssertionError:" in str(e)) | ||
|
||
try: | ||
assert_not_equal(0, 0) | ||
except e: | ||
assert_true("test_assertion.mojo:48:25: AssertionError:" in str(e)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved here to remove dependency on line numbers and future changes
I wish someone would just add the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I just added something very similar to one of my repos, except for List[List[Int]]
. I can see this being useful for the time being. It would simplify tests in /test_list.mojo
One suggestion:
Add a TODO to remove this once we have associated types or more powerful conditional conformance that can infer conformance based on methods which use a inferred parameter to conditionally conform to the trait.
Cool. There are two PRs atm that would benefit from this as well.
There's a comment right above the other overloads, will modify that.
I actually opened a discussion relevant to this issue, #3357, if you have any opinions :) |
Oh yeah, i remember a comment in there somewhere, i guess my PR which removes the need for the other overloads hasn't gotten merged yet. |
Signed-off-by: Joshua James Venter <[email protected]>
b2f8597
to
c044d04
Compare
!sync |
✅🟣 This contribution has been merged 🟣✅ Your pull request has been merged to the internal upstream Mojo sources. It will be reflected here in the Mojo repository on the nightly branch during the next Mojo nightly release, typically within the next 24-48 hours. We use Copybara to merge external contributions, click here to learn more. |
Landed in 1fad4ad! Thank you for your contribution 🎉 |
[External] [stdlib] assertion overloads for `List` Add `assert_equal` and `assert_not_equal` overload for `List` comparisons, a nice-to-have in a few places. Most probably a temporary solution under the current trait system. Co-authored-by: Joshua James Venter <[email protected]> Closes #3372 MODULAR_ORIG_COMMIT_REV_ID: 73743fb7a84f578717998c6359ba6c07cd6709ab
Add
assert_equal
andassert_not_equal
overload forList
comparisons, a nice-to-have in a few places. Most probably a temporary solution under the current trait system.