Bug 1476053 - Add an option not to store state when running wpt-update, r=jdm draft
authorJames Graham <james@hoppipolla.co.uk>
Tue, 10 Jul 2018 10:09:43 +0100
changeset 821564 54151b980e8896ea80fa07c5670631cfd00094b7
parent 821479 143984185dcece46031c970179ddea4837a6c01d
child 821565 277d44202732ccd48e2e0f44555d60b3a7902cd9
push id117141
push userbmo:james@hoppipolla.co.uk
push dateMon, 23 Jul 2018 18:17:38 +0000
reviewersjdm
bugs1476053
milestone63.0a1
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
testing/web-platform/mach_commands.py
testing/web-platform/tests/tools/wptrunner/wptrunner/update/state.py
testing/web-platform/tests/tools/wptrunner/wptrunner/update/update.py
testing/web-platform/tests/tools/wptrunner/wptrunner/wptcommandline.py
--- a/testing/web-platform/mach_commands.py
+++ b/testing/web-platform/mach_commands.py
@@ -134,16 +134,18 @@ class WebPlatformTestsUpdater(MozbuildOb
         import update
         from update import updatecommandline
 
         if kwargs["config"] is None:
             kwargs["config"] = os.path.join(self.topsrcdir, 'testing', 'web-platform', 'wptrunner.ini')
         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})
 
         try:
             update.run_update(logger, **kwargs)
         except Exception:
             import pdb
             import traceback
--- a/testing/web-platform/tests/tools/wptrunner/wptrunner/update/state.py
+++ b/testing/web-platform/tests/tools/wptrunner/wptrunner/update/state.py
@@ -1,33 +1,27 @@
 import os
 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:
             logger.debug("Existing state found")
             return rv
 
         logger.debug("No existing state found")
         return object.__new__(cls, logger)
 
     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.
         """
 
         if hasattr(self, "_data"):
             return
 
@@ -35,59 +29,32 @@ class State(object):
         self._logger = logger
         self._index = 0
 
     def __getstate__(self):
         rv = self.__dict__.copy()
         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
 
         :param init_values: List of variable names in the current state dict to copy
                             into the new state dict."""
 
         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)
         else:
             self._data[self._index][key] = value
             self.save()
 
     def __getattr__(self, key):
@@ -104,16 +71,70 @@ class State(object):
     def update(self, items):
         """Add a dictionary of {name: value} pairs to the state"""
         self._data[self._index].update(items)
         self.save()
 
     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
         self.init_values = init_values
 
     def __enter__(self):
         if len(self.state._data) == self.state._index + 1:
             # This is the case where there is no stored state
--- a/testing/web-platform/tests/tools/wptrunner/wptrunner/update/update.py
+++ b/testing/web-platform/tests/tools/wptrunner/wptrunner/update/update.py
@@ -1,22 +1,24 @@
 import os
 import sys
 
 from metadata import MetadataUpdateRunner
 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."""
 
     provides = ["sync", "paths", "metadata_path", "tests_path"]
 
     def create(self, state):
         state.sync = {"remote_url": state.kwargs["remote_url"],
                       "branch": state.kwargs["branch"],
@@ -117,17 +119,20 @@ class WPTUpdate(object):
 
         if not kwargs["sync"]:
             setup_paths(self.serve_root)
         else:
             if os.path.exists(kwargs["sync_path"]):
                 # 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
 
     def run(self, **kwargs):
         if self.kwargs["abort"]:
             self.abort()
             return exit_clean
 
--- a/testing/web-platform/tests/tools/wptrunner/wptrunner/wptcommandline.py
+++ b/testing/web-platform/tests/tools/wptrunner/wptrunner/wptcommandline.py
@@ -549,18 +549,22 @@ def create_parser_update(product_choices
                         help="Don't create a VCS commit containing the changes.")
     parser.add_argument("--sync", dest="sync", action="store_true", default=False,
                         help="Sync the tests with the latest from upstream (implies --patch)")
     parser.add_argument("--ignore-existing", action="store_true",
                         help="When updating test results only consider results from the logfiles provided, not existing expectations.")
     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="*",
                         help="List of glob-style paths to include which would otherwise be excluded when syncing tests")
     parser.add_argument("--extra-property", action="append", default=[],
                         help="Extra property from run_info.json to use in metadata update")
     # Should make this required iff run=logfile
     parser.add_argument("run_log", nargs="*", type=abs_path,