Skip to content

[stdlib] Add convenience pixi testing and format config#5119

Closed
martinvuyk wants to merge 1 commit intomodular:mainfrom
martinvuyk:add-pixi-convenience-config
Closed

[stdlib] Add convenience pixi testing and format config#5119
martinvuyk wants to merge 1 commit intomodular:mainfrom
martinvuyk:add-pixi-convenience-config

Conversation

@martinvuyk
Copy link
Copy Markdown
Contributor

Add convenience pixi testing and format config

@martinvuyk martinvuyk requested review from a team as code owners July 30, 2025 15:42
@martinvuyk martinvuyk force-pushed the add-pixi-convenience-config branch from 305f74a to e595555 Compare July 30, 2025 15:43

exec "$REPO_ROOT"/bazelw test //mojo/stdlib/test/...

FILTER="stdlib/test"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Question Do we want to maintain this script even at all now that we have e591f3e? People just can use bazelw directly and specify their subdirectory of interest.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yea this is mainly meant as a courtesy option for transitioning to the bazel build. Personally I'd rather people just use bazel directly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I prefer using the pixi task. Faster to type as well.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1 as pixi task. A lot less users have used bazel and don't want to necessarily learn yet another thing. Just bc something is used internally doesn't have to leak into the user-space so it's great to have it hidden as much as possible.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

no pref on this script or not since it's tiny, but @ehsanmok note that most things in the repo are bazel only, this script only affects the stdlib

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think there's probably a bit too many potential workflows for that to be very understandable

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1, the second someone says "I want to run a subset of the stdlib tests" we have to come back and update this workflow to pipe things through. I think a quick start guide to Bazel, and maybe some convenience aliases like we have internally (like bb and br) would help. All this is doing currently is just adding another layer of indirection.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I understand there're nuances. My point is to lower the barrier to entry pixi tasks are perfect. Sure, more advanced stuff leaks bazel requirements.

Copy link
Copy Markdown
Contributor Author

@martinvuyk martinvuyk Jul 30, 2025

Choose a reason for hiding this comment

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

"I want to run a subset of the stdlib tests"

That is the motivation for this. Now somebody can run a subdirectory. If we want to get down to the layer of running single files again, then yeah it could be too much of a hassle for you guys (although I would very much like having it).

Copy link
Copy Markdown
Member

@JoeLoser JoeLoser Jul 30, 2025

Choose a reason for hiding this comment

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

Each test file should have its own test target you can use bazel query to find and run directly FYI.

@JoeLoser JoeLoser requested review from a team and keith July 30, 2025 17:19
Copy link
Copy Markdown
Contributor

@keith keith left a comment

Choose a reason for hiding this comment

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

i think we should land and keep this script afloat while it's easy like this

@keith keith requested a review from JoeLoser July 30, 2025 18:39
@martinvuyk
Copy link
Copy Markdown
Contributor Author

Each test file should have its own test target you can use bazel query to find and run directly FYI.

And if I find a way to plug this into the script i.e. if the arg is a file execute that command, would you guys be okay with integrating it in the future to this script?

@JoeLoser
Copy link
Copy Markdown
Member

!sync

@modularbot modularbot added the imported-internally Signals that a given pull request has been imported internally. label Jul 30, 2025
@martinvuyk martinvuyk force-pushed the add-pixi-convenience-config branch from e595555 to 6f5879a Compare July 30, 2025 19:12
Signed-off-by: martinvuyk <martin.vuyklop@gmail.com>
@martinvuyk martinvuyk force-pushed the add-pixi-convenience-config branch from 6f5879a to 4eb6cb2 Compare July 30, 2025 19:12
@martinvuyk
Copy link
Copy Markdown
Contributor Author

Each test file should have its own test target you can use bazel query to find and run directly FYI.

Thanks @JoeLoser, just added the ability to test specific files as well :D

@modularbot
Copy link
Copy Markdown
Collaborator

✅🟣 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 main 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.

@modularbot modularbot added the merged-internally Indicates that this pull request has been merged internally label Jul 30, 2025
@JoeLoser
Copy link
Copy Markdown
Member

Each test file should have its own test target you can use bazel query to find and run directly FYI.

Thanks @JoeLoser, just added the ability to test specific files as well :D

Can you open a new PR for that functionality? The subdir-only part landed already internally. Thanks!

@modularbot
Copy link
Copy Markdown
Collaborator

Landed in dd8ac26! Thank you for your contribution 🎉

@github-actions github-actions bot locked and limited conversation to collaborators Jul 31, 2025
@martinvuyk martinvuyk deleted the add-pixi-convenience-config branch July 31, 2025 18:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

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

Development

Successfully merging this pull request may close these issues.

6 participants