Skip to content

Conversation

msolianko
Copy link
Collaborator

b/219056504

Fixed tests where the expected newline was LF (\n) but some files and data contained CRLF (\r\n). In some cases, the code was modified to replace \r\n with \n for normalization. In other instances, where a string with \n was expected, the expected string was adjusted by removing that character, and the actual output underwent whitespace collapsing (removing \r\n).

@msolianko msolianko requested review from andrewsavage1 and amurovanyi and removed request for andrewsavage1 March 27, 2025 09:20
b/219056504

Fixed tests where the expected newline was LF (\n) but some
files and data contained CRLF (\r\n). In some cases, the code
was modified to replace \r\n with \n for normalization. In other
instances, where a string with \n was expected, the expected
string was adjusted by removing that character, and the actual
output underwent whitespace collapsing (removing \r\n).
Copy link
Collaborator

@amurovanyi amurovanyi left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@TyHolc TyHolc left a comment

Choose a reason for hiding this comment

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

LGTM, just working on getting those 3 failed builds to pass. It looks like they should be unrelated to this change.

@TyHolc
Copy link
Contributor

TyHolc commented Apr 1, 2025

Looks like there is a single failed test that might be related to this change:

[ RUN      ] All/CertVerifyProcInternalTest.ExtraneousMD5RootCert/CertVerifyProcBuiltin
[/3895:ERROR:cert_verify_proc_builtin.cc(624)] No net_fetcher for performing AIA chasing.
[/3895:ERROR:cert_verify_proc_builtin.cc(624)] No net_fetcher for performing AIA chasing.
[/3895:ERROR:cert_verify_proc_builtin.cc(702)] CertVerifyProcBuiltin for 127.0.0.1 failed:
----- Certificate i=0 (CN=127.0.0.1,O=Test CA,L=Mountain View,ST=California,C=US) -----
ERROR: Time is after notAfter


../../net/cert/cert_verify_proc_unittest.cc:972: Failure
Value of: error
Expected: net::OK
  Actual: -201, net::ERR_CERT_DATE_INVALID
[  FAILED  ] All/CertVerifyProcInternalTest.ExtraneousMD5RootCert/CertVerifyProcBuiltin, where GetParam() = 2 (0 ms)

Not sure if that is just a coincidentally failing test or if some of the extra string trimming broke the expiration date check for the cert in that test.

@TyHolc TyHolc merged commit 33fd038 into youtube:25.lts.1+ Apr 2, 2025
264 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants