Fix clear old sessions for local (BugFix) (canonical#1708)

* Fix top level args parsing in checkbox local

Minor: this fixes them all, including the debug level

* Minor: remove validator as it is wrong

last_job_start_time is mostly None as whenever a job finishes
it will be none

* Test the fixed clear-old-sessions message

Minor: also fix metabox as cmd was overwritten when a manifest was provided

* Fix the crash in remote as well

* Fix and add new unittest

* Test also args

* Also test the utils function

* Python3.5 :(

* Forgotten tool to corrupt sessions

* Implement feedback

* python3.5 :(

* Fix interactive comand result fetching (infra) (canonical#1734)

* Result fetching for run_cmd

* Give the result half a second to materialize

* Add result to the ExecuteResult

* Preserve all fields on check

* Wrong check called

* Check is not a property

* Trace metabox output

* Rollback the binary load to test, make result array

* Set the cmd to none in non-interactive for now

* Revert trace logging

* Purge debug files

* Remove pointless append

* Use reference in the url comment

Co-authored-by: Pierre Equoy <pierre.equoy@canonical.com>

---------

Co-authored-by: Pierre Equoy <pierre.equoy@canonical.com>

---------

Co-authored-by: Pierre Equoy <pierre.equoy@canonical.com>

Author: Hook25

checkbox-ng/checkbox_ng/launcher/checkbox_cli.py

@@ -22,14 +22,14 @@
 
 import gettext
 import logging
-import os
-import subprocess
 import sys
 
+import checkbox_ng
+
 from plainbox.impl.jobcache import ResourceJobCache
 from plainbox.impl.session.assistant import SessionAssistant
 
-from checkbox_ng.config import load_configs
+from checkbox_ng.utils import set_all_loggers_level
 from checkbox_ng.launcher.subcommands import (
     Launcher,
     List,
@@ -62,6 +62,35 @@ def reset_sa(self):
         self.sa = SessionAssistant()
 
 
+def handle_top_parser(args, ctx):
+    """
+    The top level parser may not contain all args as "launcher" is inserted
+    to get a default command. Lets handle the args as stings here and unify the
+    args (from the top level parser) and ctx.args (from the sub parser)
+    """
+    if "--debug" in sys.argv:
+        logging_level = logging.DEBUG
+        logging.basicConfig(level=logging_level)
+        set_all_loggers_level(logging.DEBUG)
+        ctx.args.debug = True
+    elif "--verbose" in sys.argv or "-v" in sys.argv:
+        logging_level = logging.INFO
+        logging.basicConfig(level=logging_level)
+        set_all_loggers_level(logging.INFO)
+        ctx.args.verbose = True
+    if "--clear-cache" in sys.argv:
+        ResourceJobCache().clear()
+        ctx.args.clear_cache = True
+    if "--clear-old-sessions" in sys.argv:
+        old_sessions = [s[0] for s in ctx.sa.get_old_sessions()]
+        ctx.sa.delete_sessions(old_sessions)
+        ctx.args.clear_old_sessions = True
+    if "--version" in sys.argv:
+        print(checkbox_ng.__version__)
+        raise SystemExit(0)
+    return ctx
+
+
 def main():
     import argparse
 
@@ -105,6 +134,7 @@ def main():
             )
 
     top_parser = argparse.ArgumentParser()
+    # You must handle these args in the function above, see docstring
     top_parser.add_argument(
         "-v",
         "--verbose",
@@ -148,21 +178,7 @@ def main():
     sub_args = subcmd_parser.parse_args(sys.argv[subcmd_index + 1 :])
     sa = SessionAssistant()
     ctx = Context(sub_args, sa)
-    try:
-        socket.getaddrinfo("localhost", 443)  # 443 for HTTPS
-    except Exception:
-        pass
-    if "--clear-cache" in sys.argv:
-        ResourceJobCache().clear()
-    if "--clear-old-sessions" in sys.argv:
-        old_sessions = [s[0] for s in sa.get_old_sessions()]
-        sa.delete_sessions(old_sessions)
-    if args.verbose:
-        logging_level = logging.INFO
-        logging.basicConfig(level=logging_level)
-    if args.debug:
-        logging_level = logging.DEBUG
-        logging.basicConfig(level=logging_level)
+    ctx = handle_top_parser(args, ctx)
     return subcmd.invoked(ctx)
 
 

checkbox-ng/checkbox_ng/launcher/controller.py

@@ -41,7 +41,10 @@
 from plainbox.impl.result import MemoryJobResult
 from plainbox.impl.color import Colorizer
 from plainbox.impl.config import Configuration
-from plainbox.impl.session.resume import IncompatibleJobError
+from plainbox.impl.session.resume import (
+    IncompatibleJobError,
+    CorruptedSessionError,
+)
 from plainbox.impl.session.remote_assistant import RemoteSessionAssistant
 from plainbox.vendor import rpyc
 from checkbox_ng.resume_menu import ResumeMenu
@@ -384,6 +387,8 @@ def _resumed_session(self, session_id):
             # so that it can be treated as a normal "local" exception"
             if "plainbox.impl.session.resume.IncompatibleJobError" in str(e):
                 raise IncompatibleJobError(*e.args)
+            if "plainbox.impl.session.resume.CorruptedSessionError" in str(e):
+                raise CorruptedSessionError(*e.args)
             raise
         finally:
             self.sa.abandon_session()
@@ -408,9 +413,9 @@ def should_start_via_autoresume(self) -> bool:
         # FIXME: IncompatibleJobError is raised if the resume candidate is
         #        invalid, this is a workaround till get_resumable_sessions is
         #        fixed
-        with contextlib.suppress(IncompatibleJobError), self._resumed_session(
-            last_abandoned_session.id
-        ) as metadata:
+        with contextlib.suppress(IncompatibleJobError), contextlib.suppress(
+            CorruptedSessionError
+        ), self._resumed_session(last_abandoned_session.id) as metadata:
             app_blob = json.loads(metadata.app_blob.decode("UTF-8"))
 
             if not app_blob.get("testplan_id"):

checkbox-ng/checkbox_ng/launcher/subcommands.py

@@ -40,7 +40,10 @@
 
 from plainbox.abc import IJobResult
 from plainbox.impl.color import Colorizer
-from plainbox.impl.session.resume import IncompatibleJobError
+from plainbox.impl.session.resume import (
+    IncompatibleJobError,
+    CorruptedSessionError,
+)
 from plainbox.impl.execution import UnifiedRunner
 from plainbox.impl.highlevel import Explorer
 from plainbox.impl.result import MemoryJobResult
@@ -378,7 +381,7 @@ def _should_autoresume_last_run(self, resume_candidates):
                 if job_state.job.plugin != "shell":
                     return False
                 return True
-        except IncompatibleJobError as ije:
+        except (CorruptedSessionError, IncompatibleJobError) as ije:
             # last resumable session is incompatible, produce a helpful log
             _logger.error(
                 "Checkbox tried to resume last session (%s), but the "
@@ -420,6 +423,8 @@ def _auto_resume_session(self, resume_candidates):
                 return True
             else:
                 raise RuntimeError("Requested session is not resumable!")
+        elif self.ctx.args.clear_old_sessions:
+            return False
         elif self._should_autoresume_last_run(resume_candidates):
             last_session = resume_candidates[0]
             self._resume_session(last_session.id, None)

checkbox-ng/checkbox_ng/launcher/test_checkbox_cli.py

@@ -19,7 +19,10 @@
 from collections import namedtuple
 from unittest import TestCase, mock
 
-from checkbox_ng.launcher.checkbox_cli import main
+from checkbox_ng.launcher.checkbox_cli import (
+    main,
+    handle_top_parser,
+)
 
 
 class CheckboxCliTests(TestCase):
@@ -47,3 +50,46 @@ def test_launcher_ok(
 
         self.assertTrue(launcher_mock.called)
         self.assertTrue(launcher_mock.invoked.called)
+
+
+@mock.patch("checkbox_ng.launcher.checkbox_cli.logging", new=mock.MagicMock())
+class TestHandleTopParser(TestCase):
+
+    @mock.patch("sys.argv", ["--debug"])
+    @mock.patch("checkbox_ng.launcher.checkbox_cli.set_all_loggers_level")
+    def test_debug_flag(self, set_all_loggers_level_mock):
+        ctx = mock.MagicMock()
+        result = handle_top_parser(None, ctx)
+        self.assertTrue(result.args.debug)
+        self.assertTrue(set_all_loggers_level_mock.called)
+
+    @mock.patch("sys.argv", ["--verbose"])
+    @mock.patch("checkbox_ng.launcher.checkbox_cli.set_all_loggers_level")
+    def test_verbose_flag(self, set_all_loggers_level_mock):
+        ctx = mock.MagicMock()
+        result = handle_top_parser(None, ctx)
+        self.assertTrue(result.args.verbose)
+        self.assertTrue(set_all_loggers_level_mock.called)
+
+    @mock.patch("sys.argv", ["--clear-cache"])
+    @mock.patch("checkbox_ng.launcher.checkbox_cli.ResourceJobCache")
+    def test_clear_cache(self, mock_cache):
+        ctx = mock.MagicMock()
+        result = handle_top_parser(None, ctx)
+        self.assertTrue(mock_cache().clear.called)
+        self.assertTrue(result.args.clear_cache)
+
+    @mock.patch("sys.argv", ["--clear-old-sessions"])
+    def test_clear_old_sessions(self):
+        ctx = mock.MagicMock()
+        ctx.sa.get_old_sessions.return_value = [("session1",)]
+        result = handle_top_parser(None, ctx)
+        ctx.sa.delete_sessions.assert_called_with(["session1"])
+        self.assertTrue(result.args.clear_old_sessions)
+
+    @mock.patch("sys.argv", ["--version"])
+    def test_version_flag(self):
+        ctx = mock.MagicMock()
+        with self.assertRaises(SystemExit) as cm:
+            handle_top_parser(None, ctx)
+        self.assertEqual(cm.exception.code, 0)

checkbox-ng/checkbox_ng/launcher/test_subcommands.py

@@ -724,14 +724,30 @@ def test__auto_resume_session_from_ctx_unknown_session(self):
     def test__auto_resume_session_autoresume(self):
         self_mock = MagicMock()
         resume_candidate_mock = MagicMock(id="session_to_resume")
+        # session id wasn't provided directly via the cli
         self_mock.ctx.args.session_id = None
+        # --clear-old-sessions wasn't used
+        self_mock.ctx.args.clear_old_sessions = False
         self_mock._should_autoresume_last_run.return_value = True
 
         self.assertTrue(
             Launcher._auto_resume_session(self_mock, [resume_candidate_mock])
         )
         self.assertTrue(self_mock._resume_session.called)
 
+    def test__auto_resume_session_no_autoresume_on_clear(self):
+        self_mock = MagicMock()
+        resume_candidate_mock = MagicMock(id="session_to_resume")
+        # session id wasn't provided directly via the cli
+        self_mock.ctx.args.session_id = None
+        # --clear-old-sessions was used, so we don't autoresume
+        self_mock.ctx.args.clear_old_sessions = True
+
+        self.assertFalse(
+            Launcher._auto_resume_session(self_mock, [resume_candidate_mock])
+        )
+        self.assertFalse(self_mock._resume_session.called)
+
     def test__auto_resume_session_no_autoresume(self):
         self_mock = MagicMock()
         resume_candidate_mock = MagicMock(id="session_to_resume")

checkbox-ng/checkbox_ng/test_utils.py

@@ -17,7 +17,7 @@
 # along with Checkbox.  If not, see <http://www.gnu.org/licenses/>.
 
 import unittest
-from checkbox_ng.utils import newline_join
+from checkbox_ng.utils import newline_join, set_all_loggers_level
 
 """Tests for checkbox_ng.utils"""
 
@@ -43,3 +43,27 @@ def test_newline_join_with_single_argument(self):
     def test_newline_join_with_no_arguments(self):
         expected_result = ""
         self.assertEqual(newline_join(""), expected_result)
+
+
+class TestSetAllLoggersLevel(unittest.TestCase):
+    @unittest.mock.patch("checkbox_ng.utils.logging")
+    def test_set_all_loggers_level(self, logging_mock):
+        class FakeLogger:
+            def __init__(self, name, level):
+                self.name = name
+                self.level = level
+
+            def setLevel(self, level):
+                self.level = level
+
+        logging_mock.root.manager.loggerDict = {
+            "some": FakeLogger("some", 10),
+            "other": FakeLogger("other", 230),
+        }
+        set_all_loggers_level(0)
+        self.assertTrue(
+            all(
+                x.level == 0
+                for x in logging_mock.root.manager.loggerDict.values()
+            )
+        )

checkbox-ng/checkbox_ng/utils.py

@@ -20,12 +20,25 @@
 Generic utility functions.
 """
 import json
+import logging
 import textwrap
 import datetime
 
 from plainbox.impl.color import Colorizer
 
 
+def set_all_loggers_level(level):
+    """
+    Sets the level of loggers already instantiated
+    """
+    logging.root.setLevel(level)
+    for logger in logging.root.manager.loggerDict.values():
+        try:
+            logger.setLevel(level)
+        except AttributeError:
+            pass
+
+
 def newline_join(head: str, *tail: str) -> str:
     """
     Join strings with newlines.

checkbox-ng/plainbox/impl/session/resume.py

@@ -491,6 +491,7 @@ def _restore_SessionState_metadata(cls, metadata, session_repr):
             session_repr["metadata"],
             key="last_job_start_time",
             value_type=float,
+            value_none=True,
         )