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.

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] Clean up b64encode (2/N) #3746

Closed
wants to merge 2 commits into from

Conversation

soraros
Copy link
Contributor

@soraros soraros commented Nov 5, 2024

No description provided.

@soraros soraros marked this pull request as ready for review November 5, 2024 22:04
@soraros soraros requested a review from a team as a code owner November 5, 2024 22:04
stdlib/src/math/math.mojo Outdated Show resolved Hide resolved
@JoeLoser JoeLoser self-assigned this Nov 14, 2024
@JoeLoser
Copy link
Collaborator

!sync

@modularbot modularbot added the imported-internally Signals that a given pull request has been imported internally. label Nov 14, 2024
@modularbot
Copy link
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 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.

@modularbot modularbot added merged-internally Indicates that this pull request has been merged internally merged-externally Merged externally in public mojo repo labels Nov 14, 2024
modularbot pushed a commit that referenced this pull request Nov 15, 2024
[External] [stdlib] Clean up `b64encode` (2/N)

Co-authored-by: soraros <[email protected]>
Closes #3746
MODULAR_ORIG_COMMIT_REV_ID: e5bf916a6cc953c18bcb23b482de4a86778d5f52
@modularbot
Copy link
Collaborator

Landed in 7edd561! Thank you for your contribution 🎉

@modularbot modularbot closed this Nov 15, 2024
@soraros soraros deleted the cleanup-b64 branch November 15, 2024 06:34
@soraros
Copy link
Contributor Author

soraros commented Nov 15, 2024

@JoeLoser It seems that this PR was reverted in 174104c. I can fix the code if the team could share the internal failing tests.

@JoeLoser
Copy link
Collaborator

JoeLoser commented Nov 16, 2024

@JoeLoser It seems that this PR was reverted in 174104c. I can fix the code if the team could share the internal failing tests.

Hey @soraros, thanks for the ping. This was the (indeterminate it seemed) failing test on an m7i instance:

FAIL: Mojo Standard Library :: base64/test_base64.mojo (11 of 128)
******************** TEST 'Mojo Standard Library :: base64/test_base64.mojo' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
Unhandled exception caught during execution: At /__w/modular/modular/stdlib/test/base64/test_base64.mojo:29:17: AssertionError: `left == right` comparison failed:
   left: dGhlIHF1aWNrIGJ=============================================
  right: dGhlIHF1aWNrIGJyb3duIGZveCBqdW1wcyBvdmVyIHRoZSBsYXp5IGRvZw==

--
Command Output (stderr):
--
RUN: at line 13: mojo -D ASSERT=all /__w/modular/modular/stdlib/test/base64/test_base64.mojo
+ mojo -D ASSERT=all /__w/modular/modular/stdlib/test/base64/test_base64.mojo
mojo: error: execution exited with a non-zero result: 1

Note that it passed in our pre-submit checks (which we also run on m7i), but then later was sporadically failing throughout the day, which is why we reverted to get back to a known stable state to unblock the nightlies. I haven't triaged it further/looked into the failure, but @Ahajha may have some more info.

@Ahajha
Copy link
Contributor

Ahajha commented Nov 16, 2024

I didn't look into the error much. My best guess is that it's indeterminate, but that seems odd for this kind of test. It's also possible this had some sort of collision with another unrelated change. Perhaps rebasing this PR on main and rerunning CI might give us some insight.

msaelices pushed a commit to msaelices/mojo that referenced this pull request Nov 20, 2024
[External] [stdlib] Clean up `b64encode` (2/N)

Co-authored-by: soraros <[email protected]>
Closes modularml#3746
MODULAR_ORIG_COMMIT_REV_ID: e5bf916a6cc953c18bcb23b482de4a86778d5f52
msaelices pushed a commit to msaelices/mojo that referenced this pull request Nov 22, 2024
[External] [stdlib] Clean up `b64encode` (2/N)

Co-authored-by: soraros <[email protected]>
Closes modularml#3746
MODULAR_ORIG_COMMIT_REV_ID: e5bf916a6cc953c18bcb23b482de4a86778d5f52

Signed-off-by: Manuel Saelices <[email protected]>
msaelices pushed a commit to msaelices/mojo that referenced this pull request Nov 22, 2024
[External] [stdlib] Clean up `b64encode` (2/N)

Co-authored-by: soraros <[email protected]>
Closes modularml#3746
MODULAR_ORIG_COMMIT_REV_ID: e5bf916a6cc953c18bcb23b482de4a86778d5f52

Signed-off-by: Manuel Saelices <[email protected]>
modularbot pushed a commit that referenced this pull request Dec 17, 2024
[External] [stdlib] Clean up `b64encode` (2/N)

Co-authored-by: soraros <[email protected]>
Closes #3746
MODULAR_ORIG_COMMIT_REV_ID: e5bf916a6cc953c18bcb23b482de4a86778d5f52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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.

4 participants