This is an automated email from the ASF dual-hosted git repository.

zuston pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/uniffle.git


The following commit(s) were added to refs/heads/master by this push:
     new 47ce1355c fix(test): Fix test about nvme mount for 
LocalStorageManagerTest (#2416)
47ce1355c is described below

commit 47ce1355ca71a509f36ce076b745ecac5099ea46
Author: Andras Salamon <[email protected]>
AuthorDate: Mon Mar 24 03:21:54 2025 +0100

    fix(test): Fix test about nvme mount for LocalStorageManagerTest (#2416)
    
    ### What changes were proposed in this pull request?
    
    When I run the unit tests on the `/dev/nvme0n1p6` filesystem, Linux 
incorrectly assumes that this in a HDD. It tries to read info from 
`/sys/block/nvme0n1p/queue/rotational` but it should read info from 
`/sys/block/nvme0n1/queue/rotational`.
    
    The method `getDeviceName` correctly removes the numbers at the end from 
`sda1` to create a device name `sda` but it does not handle nvme devices 
correctly, mount point  `nvme0n1p6` should return `nvme0n1` as device name, we 
need to remove the `p6` part.
    
    ### Why are the changes needed?
    
    ```
    [INFO] Running org.apache.uniffle.server.storage.LocalStorageManagerTest
    [ERROR] Tests run: 7, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 
2.879 s <<< FAILURE! - in 
org.apache.uniffle.server.storage.LocalStorageManagerTest
    [ERROR] testGetLocalStorageInfo  Time elapsed: 0.091 s  <<< FAILURE!
    org.opentest4j.AssertionFailedError: expected: <SSD> but was: <HDD>
            at org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55)
            at 
org.junit.jupiter.api.AssertionUtils.failNotEqual(AssertionUtils.java:62)
            at 
org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
            at 
org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:177)
            at 
org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1141)
            at 
org.apache.uniffle.server.storage.LocalStorageManagerTest.testGetLocalStorageInfo(LocalStorageManagerTest.java:334)
    ```
    
    ### Does this PR introduce _any_ user-facing change?
    No.
    
    ### How was this patch tested?
    UT
---
 .../apache/uniffle/storage/common/DefaultStorageMediaProvider.java   | 5 ++++-
 .../uniffle/storage/common/DefaultStorageMediaProviderTest.java      | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git 
a/storage/src/main/java/org/apache/uniffle/storage/common/DefaultStorageMediaProvider.java
 
b/storage/src/main/java/org/apache/uniffle/storage/common/DefaultStorageMediaProvider.java
index ff926eadf..b2ad3f706 100644
--- 
a/storage/src/main/java/org/apache/uniffle/storage/common/DefaultStorageMediaProvider.java
+++ 
b/storage/src/main/java/org/apache/uniffle/storage/common/DefaultStorageMediaProvider.java
@@ -102,9 +102,12 @@ public class DefaultStorageMediaProvider implements 
StorageMediaProvider {
 
   @VisibleForTesting
   static String getDeviceName(String mountPoint) {
-    // mountPoint would be /dev/sda1, /dev/vda1, rootfs, etc.
+    // mountPoint would be /dev/sda1, /dev/vda1, /dev/nvme0n1p6, rootfs, etc.
     int separatorIndex = mountPoint.lastIndexOf(File.separator);
     String deviceName = separatorIndex > -1 ? 
mountPoint.substring(separatorIndex + 1) : mountPoint;
+    if (deviceName.startsWith("nvme") && deviceName.contains("p")) {
+      return deviceName.substring(0, deviceName.lastIndexOf("p"));
+    }
     return StringUtils.stripEnd(deviceName, NUMBERIC_STRING);
   }
 }
diff --git 
a/storage/src/test/java/org/apache/uniffle/storage/common/DefaultStorageMediaProviderTest.java
 
b/storage/src/test/java/org/apache/uniffle/storage/common/DefaultStorageMediaProviderTest.java
index d7cac212d..82fe98359 100644
--- 
a/storage/src/test/java/org/apache/uniffle/storage/common/DefaultStorageMediaProviderTest.java
+++ 
b/storage/src/test/java/org/apache/uniffle/storage/common/DefaultStorageMediaProviderTest.java
@@ -52,6 +52,7 @@ public class DefaultStorageMediaProviderTest {
     assertEquals("rootfs", 
DefaultStorageMediaProvider.getDeviceName("rootfs"));
     assertEquals("sda", 
DefaultStorageMediaProvider.getDeviceName("/dev/sda1"));
     assertEquals("cl-home", 
DefaultStorageMediaProvider.getDeviceName("/dev/mapper/cl-home"));
+    assertEquals("nvme0n1", 
DefaultStorageMediaProvider.getDeviceName("/dev/nvme0n1p6"));
   }
 
   @Test

Reply via email to