Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix mocks that cause tests to fail outside of containers #1276

Merged
merged 2 commits into from
Aug 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1266,6 +1266,7 @@ def perform():
target_iso = next(api.consume(TargetOSInstallationImage), None)
with mounting.mount_upgrade_iso_to_root_dir(overlay.target, target_iso):

# TODO: this is out of tests completely
setup_target_rhui_access_if_needed(context, indata)

target_repoids = _gather_target_repositories(context, indata, prod_cert_path)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1213,6 +1213,7 @@ def test_perform_ok(monkeypatch):
monkeypatch.setattr(overlaygen, 'create_source_overlay', MockedMountingBase)
monkeypatch.setattr(userspacegen, '_gather_target_repositories', lambda *x: repoids)
monkeypatch.setattr(userspacegen, '_create_target_userspace', lambda *x: None)
monkeypatch.setattr(userspacegen, 'setup_target_rhui_access_if_needed', lambda *x: None)
monkeypatch.setattr(userspacegen.api, 'current_actor', CurrentActorMocked())
monkeypatch.setattr(userspacegen.api, 'produce', produce_mocked())
monkeypatch.setattr(repofileutils, 'get_repodirs', lambda: ['/etc/yum.repos.d'])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,10 @@
from leapp import reporting
from leapp.exceptions import StopActorExecution
from leapp.libraries.actor import updategrubcore
from leapp.libraries.common import testutils
from leapp.libraries.common import grub, testutils
from leapp.libraries.common.config import architecture
from leapp.libraries.stdlib import CalledProcessError
from leapp.models import FirmwareFacts
from leapp.reporting import Report

UPDATE_OK_TITLE = 'GRUB core successfully updated'
UPDATE_FAILED_TITLE = 'GRUB core update failed'
Expand Down Expand Up @@ -107,14 +106,13 @@ def test_update_grub_nogrub_system_ibmz(monkeypatch):


def test_update_grub_nogrub_system(monkeypatch):
def raise_call_oserror(dummy):
# Note: grub2-probe is enough right now. If the implementation is changed,
# the test will most likely start to fail and better mocking will be needed.
raise OSError('File not found: grub2-probe')
def get_grub_devices_mocked():
# this is not very well documented, but the grub.get_grub_devices function raises a StopActorExecution on error
# (whether that's caused by determining root partition or determining the block device a given partition is on
raise StopActorExecution()

monkeypatch.setattr(grub, 'get_grub_devices', get_grub_devices_mocked)
monkeypatch.setattr(reporting, 'create_report', testutils.create_report_mocked())
monkeypatch.setattr(updategrubcore, 'run', run_mocked(raise_err=True, raise_callback=raise_call_oserror))

msgs = [FirmwareFacts(firmware='bios')]
curr_actor_mocked = testutils.CurrentActorMocked(arch=architecture.ARCH_X86_64, msgs=msgs)
monkeypatch.setattr(updategrubcore.api, 'current_actor', curr_actor_mocked)
Expand Down
7 changes: 4 additions & 3 deletions repos/system_upgrade/common/libraries/grub.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def blk_dev_from_partition(partition):
api.current_logger().warning(
'Could not get parent device of {} partition'.format(partition)
)
raise StopActorExecution()
raise StopActorExecution() # TODO: return some meaningful value/error
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 ➕ that's bug introduced basically when the private actor's library has been moved to shared library instead. Checking the library, it should be fixed everywhere, but let's keep it for another PR. Good catch!

# lsblk "-s" option prints dependencies in inverse order, so the parent device will always
# be the last or the only device.
# Command result example:
Expand All @@ -55,14 +55,14 @@ def get_boot_partition():
api.current_logger().warning(
'Could not get name of underlying /boot partition'
)
raise StopActorExecution()
raise StopActorExecution() # TODO: return some meaningful value/error
except OSError:
api.current_logger().warning(
'Could not get name of underlying /boot partition:'
' grub2-probe is missing.'
' Possibly called on system that does not use GRUB2?'
)
raise StopActorExecution()
raise StopActorExecution() # TODO: return some meaningful value/error
boot_partition = result['stdout'].strip()
api.current_logger().info('/boot is on {}'.format(boot_partition))
return boot_partition
Expand All @@ -77,6 +77,7 @@ def get_grub_devices():
:return: Devices where GRUB is located
:rtype: list
"""
# TODO: catch errors and return meaningful value/error instead of StopActorExecution
boot_device = get_boot_partition()
devices = []
if mdraid.is_mdraid_dev(boot_device):
Expand Down