Skip to content

nvme basic test fix for disk count match #4437

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.

Sign up for GitHub

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

Draft
kanchansenlaskar wants to merge 1 commit into
base: main
Choose a base branch
from
+8 −8
Draft

nvme basic test fix for disk count match #4437

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 8 additions & 8 deletions lisa/microsoft/testsuites/nvme/nvme.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Copyright (c) Microsoft Corporation.
# Licensed under the MIT license.
import math
from typing import cast
Copy link

Copilot AI Apr 26, 2026

Choose a reason for hiding this comment

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

The PR description content appears to be entirely inside an HTML comment in the PR template, so reviewers won’t see the rationale or the before/after behavior. Please add a visible description explaining what this PR changes and why.

Copilot uses AI. Check for mistakes.

from assertpy import assert_that

Expand All @@ -16,7 +16,7 @@
from lisa.features import Nvme, NvmeSettings, Sriov
from lisa.search_space import IntRange
from lisa.sut_orchestrator.azure.platform_ import AzurePlatform
from lisa.tools import Cat, Df, Echo, Fdisk, Lscpu, Lspci, Mkfs, Mount, Nvmecli
from lisa.tools import Cat, Df, Echo, Fdisk, Lspci, Mkfs, Mount, Nvmecli
from lisa.tools.fdisk import FileSystem
from lisa.util.constants import DEVICE_TYPE_NVME, DEVICE_TYPE_SRIOV

Expand Down Expand Up @@ -66,7 +66,7 @@ class NvmeTestSuite(TestSuite):
and list nvme devices under /dev/.

4. Azure platform only, nvme devices count should equal to
actual vCPU count / 8.
expected disk count from SKU capabilities.
Comment on lines 68 to +69
Copy link

Copilot AI Apr 26, 2026

Choose a reason for hiding this comment

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

This step says "nvme devices count" but the implementation uses the namespace list length as the count being asserted later. Please clarify the wording to refer to namespaces/disks (e.g., /dev/nvme*n*) to avoid confusion about controller vs namespace counts.

Copilot uses AI. Check for mistakes.
""",
priority=1,
requirement=simple_requirement(
Expand Down Expand Up @@ -414,13 +414,13 @@ def _verify_nvme_disk(self, environment: Environment, node: Node) -> None:
).is_length(len(nvme_device_from_lspci))

# 4. Azure platform only, nvme devices count should equal to
# actual vCPU count / 8.
# expected disk count from SKU capabilities.
Comment on lines 416 to +417
Copy link

Copilot AI Apr 26, 2026

Choose a reason for hiding this comment

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

This comment says "nvme devices count" but the assertion below is on the NVMe namespaces list length. Please update the comment to match what’s being checked (namespaces/disks vs controllers/devices).

Copilot uses AI. Check for mistakes.
if isinstance(environment.platform, AzurePlatform):
lscpu_tool = node.tools[Lscpu]
thread_count = lscpu_tool.get_thread_count()
expected_count = math.ceil(thread_count / 8)
nvme_settings = cast(NvmeSettings, node.features[Nvme].get_settings())
expected_count = nvme_settings.disk_count
assert_that(nvme_namespace).described_as(
"nvme devices count should be equal to [vCPU/8]."
f"nvme devices count should be equal to expected disk count"
Copy link

Copilot AI Apr 26, 2026

Choose a reason for hiding this comment

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

The assertion is checking nvme_namespace length, but the described_as message says "nvme devices count". Please align the message with the actual subject being asserted (namespaces/disks), so failures are easier to interpret.

Suggested change
f"nvme devices count should be equal to expected disk count"
f"nvme namespace count should be equal to expected disk count"

Copilot uses AI. Check for mistakes.
f" [{expected_count}] from SKU capabilities."
).is_length(expected_count)

def _verify_nvme_function(self, node: Node, use_partitions: bool = True) -> None:
Expand Down
Loading