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

adding tests for by-id core path #931

Conversation

karolinavelkaja
Copy link
Contributor

@karolinavelkaja karolinavelkaja commented Aug 20, 2021

Original review: #539

This PR needs functionality from: https://github.com/Open-CAS/test-framework/pull/259

Signed-off-by: Karolina Rogowska karolina.rogowska@intel.com

Signed-off-by: Karolina Rogowska <karolina.rogowska@intel.com>
@karolinavelkaja karolinavelkaja added the RFC Request For Comments label Aug 23, 2021
@@ -0,0 +1,216 @@
#
# Copyright(c) 2021 Intel Corporation
Copy link
Contributor

Choose a reason for hiding this comment

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

please update to 2022

Comment on lines +56 to +58
Symlink(os.path.join(by_id_dir, item.full_path)) # parse_ls_output returns
for item in parse_ls_output(ls(by_id_dir), by_id_dir) # symlinks without path
if isinstance(item, Symlink)
Copy link
Contributor

Choose a reason for hiding this comment

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

Resolving by-id link seems to be a very common operation. I suggest to add a helper function for this in the framework.

Comment on lines +65 to +71
with TestRun.step("Start cache and add cores"):
paths_from_dmesg = []
cache = casadm.start_cache(cache_part, force=True)
for i in range(cores_number):
core_dev.partitions[i].path = selected_links[i].full_path
cache.add_core(core_dev.partitions[i])
paths_from_dmesg.append(get_added_core_path_from_dmesg())
Copy link
Contributor

Choose a reason for hiding this comment

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

there is no need to parse dmesg. "casadm -L -b" will print paths exactly as returned by OCF, without any translation.

Comment on lines 11 to 26
from api.cas import casadm, casctl, casadm_parser
from api.cas.casadm_parser import get_caches, get_cores
from api.cas.cache_config import CacheMode
from api.cas.casadm import list_caches
from api.cas.casadm_parser import get_caches, get_cores, get_cas_devices_dict
from api.cas.cli_messages import check_stdout_msg, no_caches_running
from api.cas.init_config import InitConfig
from core.test_run import TestRun
from storage_devices.disk import DiskType, DiskTypeSet, DiskTypeLowerThan
from test_utils.filesystem.file import File
from test_tools.dd import Dd
from test_tools.disk_utils import Filesystem
from test_tools.fs_utils import parse_ls_output, readlink, ls
from test_utils import fstab
from test_tools.dd import Dd
from test_utils.filesystem.file import File
from test_utils.filesystem.symlink import Symlink
from test_utils.os_utils import drop_caches, sync
from test_utils.size import Unit, Size
Copy link
Contributor

Choose a reason for hiding this comment

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

could use some cleanup, like removing casadm_parser as a whole module from imports (all the needed functions from casadm_parser are imported) or importing casadm and from casadm

Copy link
Contributor

@Deixx Deixx left a comment

Choose a reason for hiding this comment

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

Adam's comments still relevant, but this should work almost "as is"

@katlapinka katlapinka self-assigned this Sep 6, 2024
@katlapinka
Copy link
Contributor

Moved to #1518

@katlapinka katlapinka closed this Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC Request For Comments tests v24.9
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants