Replace ICU encoding detection with CED

net/ftp/ftp_directory_listing_parser_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/ftp/ftp_directory_listing_parser.h"
 
 #include "base/files/file_util.h"
 #include "base/format_macros.h"
 #include "base/path_service.h"
 #include "base/strings/string_number_conversions.h"
@@ skipping 121 matching lines @@
     {"dir-listing-ls-10", OK},
     {"dir-listing-ls-11", OK},
     {"dir-listing-ls-12", OK},
     {"dir-listing-ls-13", OK},
     {"dir-listing-ls-14", OK},
     {"dir-listing-ls-15", OK},
     {"dir-listing-ls-16", OK},
     {"dir-listing-ls-17", OK},
     {"dir-listing-ls-18", OK},
     {"dir-listing-ls-19", OK},
-    {"dir-listing-ls-20", OK},
-    {"dir-listing-ls-21", OK},
-    {"dir-listing-ls-22", OK},
+
+    // TODO(jinsukkim): Enable following 3 test cases after the fix
+    //     lands manually. See crrev.com/2168003003
+    // {"dir-listing-ls-20", OK},

[Paweł Hajdan Jr., 2016/07/22 13:22:19]

I'm worried about these tests getting disabled.

Does that mean this change breaks them?

Historically we needed DetectAllEncodings vs. just DetectEncoding.

I'm taking vacation next week. Feel free to get review from net/ OWNERS,
although I'd recommend that this concern gets addressed and no tests get
disabled. It doesn't seem trivial to fix them.

[Jinsuk Kim, 2016/07/24 22:24:46]

No quite the opposite - the 3 tests were already failing, and this CL fixes
them. I'm temporarily commented them out because the test files to update have
DOS-formatted linefeeds which Rietveld cannot handle. I'm going them put them
back once the review is completed and land manually. Updated the comment to make
the intention clearer.

[Paweł Hajdan Jr., 2016/08/08 09:00:48]

I'd like to understand this better.

These test cases were _enabled_ before. Could you explain more what do you mean
that they were failing?

Also, it seems you'd like to change the corresponding data files. Could you
explain more why and how would the contents change?

[Jinsuk Kim, 2016/08/08 11:28:25]

If you open the expected files, they are actually wrong results. They were
checked in just to make the tests pass.

+    // {"dir-listing-ls-21", OK},
+    // {"dir-listing-ls-22", OK},
+
     {"dir-listing-ls-23", OK},
     {"dir-listing-ls-24", OK},
 
     // Tests for Russian listings. The only difference between those
     // files is character encoding:
     {"dir-listing-ls-25", OK},  // UTF-8
     {"dir-listing-ls-26", OK},  // KOI8-R
     {"dir-listing-ls-27", OK},  // windows-1251
 
     {"dir-listing-ls-28", OK},  // Hylafax FTP server
@@ skipping 20 matching lines @@
 };
 
 INSTANTIATE_TEST_CASE_P(,
                         FtpDirectoryListingParserTest,
                         testing::ValuesIn(kTestParams),
                         TestName);
 
 }  // namespace
 
 }  // namespace net