Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[stdlib] Add test for InlineArray (sizeof[T] by capacity). #4125

Closed
wants to merge 2 commits into from
Closed

[stdlib] Add test for InlineArray (sizeof[T] by capacity). #4125

wants to merge 2 commits into from

Conversation

rd4com
Copy link
Contributor

@rd4com rd4com commented Mar 8, 2025

Hello,
this PR adds a small test for InlineArray:

(sizeof[ElementType]() * capacity) == sizeof[self._array]()

@rd4com
[stdlib] Add test for InlineArray (sizeof[T] by capacity).
64783be 
Hello,
this PR adds a small test for `InlineArray`:

```mojo
(sizeof[ElementType]() * capacity) == sizeof[self._array]()
```

Signed-off-by: rd4com <144297616+rd4com@users.noreply.github.com>
@rd4com rd4com force-pushed the add_tests_inlinelist branch from dba1f4c to 64783be Compare March 8, 2025 21:16
@rd4com
Implement review
db07f19 
Signed-off-by: rd4com <144297616+rd4com@users.noreply.github.com>
@soraros
Copy link
Contributor

soraros commented Mar 9, 2025

Which one do we prefer for tests these days? fn raises or def?

JoeLoser
JoeLoser approved these changes Mar 10, 2025
View reviewed changes
Copy link
Collaborator

@JoeLoser JoeLoser left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the additional testing coverage!

@JoeLoser
Copy link
Collaborator

!sync

@JoeLoser
Copy link
Collaborator

Which one do we prefer for tests these days? fn raises or def?

In general, we still prefer def, but it's easy to refactor all of the tests, so I won't reject a contribution for something minor like that.

@modularbot modularbot added imported-internally Signals that a given pull request has been imported internally. merged-externally Merged externally in public mojo repo labels Mar 10, 2025
@modularbot
Copy link
Collaborator

Landed in 8123f0f! Thank you for your contribution 🎉

@JoeLoser JoeLoser added the merged-internally Indicates that this pull request has been merged internally label Mar 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JoeLoser JoeLoser JoeLoser approved these changes

@gryznar gryznar gryznar approved these changes

Assignees

@JoeLoser JoeLoser

Labels
imported-internally Signals that a given pull request has been imported internally. merged-externally Merged externally in public mojo repo merged-internally Indicates that this pull request has been merged internally
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@rd4com @soraros @JoeLoser @modularbot @gryznar