Refactoring: Move _port_skips_test to Port class.

third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py

 # Copyright (c) 2010 Google Inc. All rights reserved.
 #
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions are
 # met:
 #
 #     * Redistributions of source code must retain the above copyright
 # notice, this list of conditions and the following disclaimer.
 #     * Redistributions in binary form must reproduce the above
 # copyright notice, this list of conditions and the following disclaimer
@@ skipping 174 matching lines @@
             if not self._tool.filesystem.exists(old_baseline):
                 _log.debug("No existing baseline for %s.", test_name)
                 continue
 
             new_baseline = self._tool.filesystem.join(port.baseline_path(), self._file_name_for_expected_result(test_name, suffix))
             if self._tool.filesystem.exists(new_baseline):
                 _log.debug("Existing baseline at %s, not copying over it.", new_baseline)
                 continue
 
             expectations = TestExpectations(port, [test_name])
+            # TODO(qyearsley): Make sure we're also skipping copying existing baselines in
+            # the case where the port only runs smoke tests and the test is a smoke test.
             if SKIP in expectations.get_expectations(test_name):
                 _log.debug("%s is skipped on %s.", test_name, port.name())
                 continue
 
             old_baselines.append(old_baseline)
             new_baselines.append(new_baseline)
 
         for i in range(len(old_baselines)):
             old_baseline = old_baselines[i]
             new_baseline = new_baselines[i]
@@ skipping 213 matching lines @@
         # This is so we remove lines for builders that skip this test, e.g. Android skips most
         # tests and we don't want to leave stray [ Android ] lines in TestExpectations..
         # This is only necessary for "webkit-patch rebaseline" and for rebaselining expected
         # failures from garden-o-matic. rebaseline-expectations and auto-rebaseline will always
         # pass the exact set of ports to rebaseline.
         for port_name in self._tool.port_factory.all_port_names():
             port = self._tool.port_factory.get(port_name)
             generic_expectations = TestExpectations(port, tests=tests, include_overrides=False)
             full_expectations = TestExpectations(port, tests=tests, include_overrides=True)
             for test in tests:
-                if self._port_skips_test(port, test, generic_expectations, full_expectations):
+                if port.skips_test(test, generic_expectations, full_expectations):
                     for test_configuration in port.all_test_configurations():
                         if test_configuration.version == port.test_configuration().version:
                             to_remove.append((test, test_configuration))
 
         for test in lines_to_remove:
             for builder in lines_to_remove[test]:
                 port = self._tool.port_factory.get_from_builder_name(builder)
                 for test_configuration in port.all_test_configurations():
                     if test_configuration.version == port.test_configuration().version:
                         to_remove.append((test, test_configuration))
 
         port = self._tool.port_factory.get()
         expectations = TestExpectations(port, include_overrides=False)
         expectations_string = expectations.remove_configurations(to_remove)
         path = port.path_to_generic_test_expectations_file()
         self._tool.filesystem.write_text_file(path, expectations_string)
 
-    @staticmethod
-    def _port_skips_test(port, test, generic_expectations, full_expectations):
-        fs = port.host.filesystem
-        if port.default_smoke_test_only():
-            smoke_test_filename = fs.join(port.layout_tests_dir(), 'SmokeTests')
-            if fs.exists(smoke_test_filename) and test not in fs.read_text_file(smoke_test_filename):
-                return True
-
-        return (SKIP in full_expectations.get_expectations(test) and
-                SKIP not in generic_expectations.get_expectations(test))

[qyearsley, 2016/10/27 00:47:11]

It seems like this should just be (SKIP in
full_expectations.get_expectations(test)) -- I'm not sure why we would also want
to check that SKIP is not in the generic expectations file expectations.

[Dirk Pranke, 2016/10/27 00:53:45]

Unfortunately, I don't remember why we did this. Your question seems reasonable;
it might've been left over from a time when things were more complicated.

-
     def _run_in_parallel(self, commands):
         if not commands:
             return {}
 
         command_results = self._tool.executive.run_in_parallel(commands)
         for _, _, stderr in command_results:
             if stderr:
                 _log.error(stderr)
 
         change_set = self._extract_expectation_line_changes(command_results)
@@ skipping 239 matching lines @@
             for test in args:
                 if test not in test_prefix_list:
                     test_prefix_list[test] = {}
                 build = Build(builder)
                 test_prefix_list[test][build] = suffixes_to_update
 
         if options.verbose:
             _log.debug("rebaseline-json: " + str(test_prefix_list))
 
         self.rebaseline(options, test_prefix_list)