Bug 1476053 - Add an option not to store state when running wpt-update, r=jdm

This adds some overhead and can be annoying since it requries explicitly aborting failed jobs.
The state storage isn't very useful for just udpating metadata (the typical gecko usecase), but
is useful for performing syncs (a typical servo usecase). Therefore add a --no-store-state option
and set it by default in the gecko mach frontend.

MozReview-Commit-ID: LhEcMkyuRHD

--HG--
extra : rebase_source : 0f5c22b2a1337f7ca370cae31514792528d234a3

testing/web-platform/mach_commands.py

@@ -140,6 +140,8 @@ class WebPlatformTestsUpdater(MozbuildObject):
         if kwargs["product"] is None:
             kwargs["product"] = "firefox"
 
+        kwargs["store_state"] = False
+
         kwargs = updatecommandline.check_args(kwargs)
         logger = update.setup_logging(kwargs, {"mach": sys.stdout})
 

testing/web-platform/tests/tools/wptrunner/wptrunner/update/state.py

@@ -3,9 +3,8 @@ import cPickle as pickle
 
 here = os.path.abspath(os.path.split(__file__)[0])
 
-class State(object):
-    filename = os.path.join(here, ".wpt-update.lock")
 
+class BaseState(object):
     def __new__(cls, logger):
         rv = cls.load(logger)
         if rv is not None:
@@ -18,11 +17,6 @@ class State(object):
     def __init__(self, logger):
         """Object containing state variables created when running Steps.
 
-        On write the state is serialized to disk, such that it can be restored in
-        the event that the program is interrupted before all steps are complete.
-        Note that this only works well if the values are immutable; mutating an
-        existing value will not cause the data to be serialized.
-
         Variables are set and get as attributes e.g. state_obj.spam = "eggs".
 
         :param parent: Parent State object or None if this is the root object.
@@ -40,23 +34,6 @@ class State(object):
         del rv["_logger"]
         return rv
 
-    @classmethod
-    def load(cls, logger):
-        """Load saved state from a file"""
-        try:
-            if not os.path.isfile(cls.filename):
-                return None
-            with open(cls.filename) as f:
-                try:
-                    rv = pickle.load(f)
-                    logger.debug("Loading data %r" % (rv._data,))
-                    rv._logger = logger
-                    rv._index = 0
-                    return rv
-                except EOFError:
-                    logger.warning("Found empty state file")
-        except IOError:
-            logger.debug("IOError loading stored state")
 
     def push(self, init_values):
         """Push a new clean state dictionary
@@ -66,23 +43,13 @@ class State(object):
 
         return StateContext(self, init_values)
 
-    def save(self):
-        """Write the state to disk"""
-        with open(self.filename, "w") as f:
-            pickle.dump(self, f)
-
     def is_empty(self):
         return len(self._data) == 1 and self._data[0] == {}
 
     def clear(self):
         """Remove all state and delete the stored copy."""
-        try:
-            os.unlink(self.filename)
-        except OSError:
-            pass
         self._data = [{}]
 
-
     def __setattr__(self, key, value):
         if key.startswith("_"):
             object.__setattr__(self, key, value)
@@ -109,6 +76,60 @@ class State(object):
     def keys(self):
         return self._data[self._index].keys()
 
+    @classmethod
+    def load(self):
+        raise NotImplementedError
+
+    def save(self):
+        raise NotImplementedError
+
+
+class SavedState(BaseState):
+    """On write the state is serialized to disk, such that it can be restored in
+       the event that the program is interrupted before all steps are complete.
+       Note that this only works well if the values are immutable; mutating an
+       existing value will not cause the data to be serialized."""
+
+    @classmethod
+    def load(cls, logger):
+        """Load saved state from a file"""
+        try:
+            if not os.path.isfile(cls.filename):
+                return None
+            with open(cls.filename) as f:
+                try:
+                    rv = pickle.load(f)
+                    logger.debug("Loading data %r" % (rv._data,))
+                    rv._logger = logger
+                    rv._index = 0
+                    return rv
+                except EOFError:
+                    logger.warning("Found empty state file")
+        except IOError:
+            logger.debug("IOError loading stored state")
+
+    def save(self):
+        """Write the state to disk"""
+        with open(self.filename, "w") as f:
+            pickle.dump(self, f)
+
+    def clear(self):
+        super(SavedState, self).clear()
+        try:
+            os.unlink(self.filename)
+        except OSError:
+            pass
+
+
+class UnsavedState(BaseState):
+    @classmethod
+    def load(cls, logger):
+        return None
+
+    def save(self):
+        return
+
+
 class StateContext(object):
     def __init__(self, state, init_values):
         self.state = state

testing/web-platform/tests/tools/wptrunner/wptrunner/update/update.py

@@ -6,12 +6,14 @@ from sync import SyncFromUpstreamRunner
 from tree import GitTree, HgTree, NoVCSTree
 
 from base import Step, StepRunner, exit_clean, exit_unclean
-from state import State
+from state import SavedState, UnsavedState
+
 
 def setup_paths(sync_path):
     sys.path.insert(0, os.path.abspath(sync_path))
     from tools import localpaths  # noqa: flake8
 
+
 class LoadConfig(Step):
     """Step for loading configuration from the ini file and kwargs."""
 
@@ -122,7 +124,10 @@ class WPTUpdate(object):
                 # If the sync path doesn't exist we defer this until it does
                 setup_paths(kwargs["sync_path"])
 
-        self.state = State(logger)
+        if kwargs["store_state"]:
+            self.state = SavedState(logger)
+        else:
+            self.state = UnsavedState(logger)
         self.kwargs = kwargs
         self.logger = logger
 

testing/web-platform/tests/tools/wptrunner/wptrunner/wptcommandline.py

@@ -554,8 +554,12 @@ def create_parser_update(product_choices=None):
     parser.add_argument("--stability", nargs="?", action="store", const="unstable", default=None,
         help=("Reason for disabling tests. When updating test results, disable tests that have "
               "inconsistent results across many runs with the given reason."))
-    parser.add_argument("--continue", action="store_true", help="Continue a previously started run of the update script")
-    parser.add_argument("--abort", action="store_true", help="Clear state from a previous incomplete run of the update script")
+    parser.add_argument("--no-store-state", action="store_false", dest="store_state",
+                        help="Store state so that steps can be resumed after failure")
+    parser.add_argument("--continue", action="store_true",
+                        help="Continue a previously started run of the update script")
+    parser.add_argument("--abort", action="store_true",
+                        help="Clear state from a previous incomplete run of the update script")
     parser.add_argument("--exclude", action="store", nargs="*",
                         help="List of glob-style paths to exclude when syncing tests")
     parser.add_argument("--include", action="store", nargs="*",