Skip to content

Commit

Permalink
Moved responsibility of system_information collection
Browse files Browse the repository at this point in the history
Now the collection is done automatically upon the first get call
if no set was done previously. This should be effectively the same as
before, the first checkpoint dumps the information, reading it, while
a non-fresh session will load a checkpoint setting before reading it

Minor: Update tests to mock/check the new path
  • Loading branch information
Hook25 committed Oct 17, 2023
1 parent fa7588d commit 82bcf81
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 30 deletions.
4 changes: 0 additions & 4 deletions checkbox-ng/plainbox/impl/session/assistant.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@
from plainbox.impl.transport import TransportError
from plainbox.impl.unit.exporter import ExporterError
from plainbox.impl.unit.unit import Unit
from plainbox.impl.session import system_information
from plainbox.vendor import morris

_logger = logging.getLogger("plainbox.session.assistant")
Expand Down Expand Up @@ -501,9 +500,6 @@ def start_new_session(
self._metadata.app_id = self._app_id
self._metadata.title = title
self._metadata.flags = {SessionMetaData.FLAG_BOOTSTRAPPING}

self._manager.state.update_system_information(system_information.collect())

self._manager.checkpoint()
self._command_io_delegate = JobRunnerUIDelegate(_SilentUI())
self._init_runner(runner_cls, runner_kwargs)
Expand Down
2 changes: 1 addition & 1 deletion checkbox-ng/plainbox/impl/session/resume.py
Original file line number Diff line number Diff line change
Expand Up @@ -1187,7 +1187,7 @@ def _restore_SessionState_system_information(self, session_state, session_repr):
"system_information"
].items()
}
session_state.update_system_information(system_information)
session_state.system_information = system_information

def _build_SessionState(self, session_repr, early_cb=None):
session_state = super()._build_SessionState(session_repr, early_cb)
Expand Down
31 changes: 16 additions & 15 deletions checkbox-ng/plainbox/impl/session/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@
from plainbox.impl.secure.qualifiers import select_jobs
from plainbox.impl.session.jobs import JobState
from plainbox.impl.session.jobs import UndesiredJobReadinessInhibitor
from plainbox.impl.session.system_information import(
collect as collect_system_information
)
from plainbox.impl.unit.job import JobDefinition
from plainbox.impl.unit.unit_with_id import UnitWithId
from plainbox.impl.unit.testplan import TestPlanUnitSupport
Expand Down Expand Up @@ -746,7 +749,9 @@ def __init__(self, unit_list):
self._resource_map = {}
self._fake_resources = False
self._metadata = SessionMetaData()
self._system_information = {}
# If unset, this is loaded via system_information
self._system_information = None

super(SessionState, self).__init__()

def trim_job_list(self, qualifier):
Expand Down Expand Up @@ -816,14 +821,17 @@ def trim_job_list(self, qualifier):
self.on_job_removed(job)
self.on_unit_removed(job)

def update_system_information(self, system_information):
"""
Update the system information with every tool and its output.
@property
def system_information(self):
if not self._system_information:
# This is a new session, we need to query this infos
self._system_information = collect_system_information()
return self._system_information

Check warning on line 829 in checkbox-ng/plainbox/impl/session/state.py

View check run for this annotation

Codecov / codecov/patch

checkbox-ng/plainbox/impl/session/state.py#L828-L829

Added lines #L828 - L829 were not covered by tests

This method simply stores the dict of system_information jobs inside
the session state.
"""
self._system_information = system_information
@system_information.setter
def system_information(self, value):
#TODO: check if system_information was already set
self._system_information = value

Check warning on line 834 in checkbox-ng/plainbox/impl/session/state.py

View check run for this annotation

Codecov / codecov/patch

checkbox-ng/plainbox/impl/session/state.py#L834

Added line #L834 was not covered by tests

def update_mandatory_job_list(self, mandatory_job_list):
"""
Expand Down Expand Up @@ -1181,13 +1189,6 @@ def set_resource_list(self, resource_id, resource_list):
"""
self._resource_map[resource_id] = resource_list

@property
def system_information(self):
"""
Dict of all system information.
"""
return self._system_information

@property
def job_list(self):
"""
Expand Down
2 changes: 1 addition & 1 deletion checkbox-ng/plainbox/impl/session/test_assistant.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def test_expected_call_sequence(self, mock_get_providers):
UsageExpectation.of(self.sa).allowed_calls)
# patch system_information to avoid the actual collection of
# system_information in tests
with mock.patch("plainbox.impl.session.assistant.system_information"):
with mock.patch("plainbox.impl.session.state.SessionState.system_information"):
# Call SessionAssistant.start_new_session()
self.sa.start_new_session("just for testing")
# SessionAssistant.start_new_session() must no longer allowed
Expand Down
20 changes: 11 additions & 9 deletions checkbox-ng/plainbox/impl/session/test_resume.py
Original file line number Diff line number Diff line change
Expand Up @@ -510,19 +510,20 @@ def test_calls_restore_SessionState_system_information(self):
self_mock = mock.MagicMock()
session_state_mock = mock.MagicMock()

session_repr = {
"system_information": {
}
}
session_repr = {"system_information": {}}

SessionResumeHelper8._restore_SessionState_system_information(
self_mock, session_state_mock, session_repr
)

session_state_mock.update_system_information.assert_called_once_with(
{}
)
def test_calls_build_SessionState(self):
with mock.patch(
"plainbox.impl.session.system_information.collect"
) as collect_mock:
_ = session_state_mock.system_information
self.assertFalse(collect_mock.called)

@mock.patch("plainbox.impl.session.system_information.collect")
def test_calls_build_SessionState(self, collect_mock):
# mock super to avoid super._build_SessionState call in this test
with mock.patch(
"plainbox.impl.session.resume.SessionResumeHelper7._build_SessionState"
Expand All @@ -531,7 +532,8 @@ def test_calls_build_SessionState(self):
session_state = session_resume_helper._build_SessionState({
"system_information" : {}
})
self.assertTrue(session_state.update_system_information.called)
self.assertNotEqual(session_state.system_information, None)
self.assertFalse(collect_mock.called)

class SessionStateResumeTests(TestCaseWithParameters):
"""
Expand Down

0 comments on commit 82bcf81

Please sign in to comment.