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
bump base64 from 0.10 to 0.21 #29804
Conversation
I looked at the dependency tree using cargo tree with -i flag and one of the dependencies is content-security-policy and other is headers and they seem reluctant to upgrade base64 based on MSRV concerns. THere is also ron (one for 0.10.1 and one for 0.13 with different ron versions) and webdriver which I did not follow the conversation for base64. |
@bors-servo r+ |
📌 Commit c554358 has been approved by |
bump base64 from 0.10 to 0.21 <!-- Please describe your changes on the following line: --> --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes partly fix #26013 - [x] These changes do not require tests because it bumps version of a dependency. From running the tests there does not seem to be any regressions (there are a lot of crashed tests but I don't think they are relevant to this change) <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
@bors-servo r+ |
📌 Commit 47bd465 has been approved by |
bump base64 from 0.10 to 0.21 <!-- Please describe your changes on the following line: --> --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes partly fix #26013 - [x] These changes do not require tests because it bumps version of a dependency. From running the tests there does not seem to be any regressions (there are a lot of crashed tests but I don't think they are relevant to this change) <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
I used |
9031b58
to
f34d56d
Compare
Test results for linux-wpt-layout-2013 from try job (#5106105343): Flaky unexpected result (23)
Stable unexpected results that are known to be intermittent (15)
Stable unexpected results (3)
|
Hmm, we've broken some tests with this change:
To reproduce, run |
This seems really weird. I did a small test and the encoded string is same byte by byte between the two versions. |
It seems that the actual assert succeeds but there is an additional warning that causes the test to fail. (Clicking on the test case shows the assert itself). Further investigation this seems the test also throws the invalidCharacterError exception. I don't see a clear reason for the code throw this exception. |
I did a few more test using the firefox devtools and the code tested does not seem to cause any problems. Is there an easy way to attach a devtools session to a test-wpt run i could not find a simple flag to be able to attach to the process from firefox ? Other than that I am at a lock here I am not sure about the cause of the regression. |
Trying to unblock this: I pushed a new commit to this branch which adjusts the configuration of the base64 engine to not require padding on decode. I think this fixes the tests. @bors-servo try=wpt |
🔨 Triggering try run (#5739125004) with platform=linux and layout=2013 |
@bors-servo try=wpt |
🔨 Triggering try run (#5739487511) with platform=linux and layout=2013 |
Test results for linux-wpt-layout-2013 from try job (#5739125004): Flaky unexpected result (12)
Stable unexpected results that are known to be intermittent (12)
Stable unexpected results (1)
|
|
Test results for linux-wpt-layout-2013 from try job (#5739487511): Flaky unexpected result (11)
Stable unexpected results that are known to be intermittent (11)
|
✨ Try run (#5739487511) succeeded. |
CI failed because of #30063. |
CI failed due to #30064. |
./mach build -d
does not report any errors./mach test-tidy
does not report any errors