From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pf1-f172.google.com (mail-pf1-f172.google.com [209.85.210.172]) by mx.groups.io with SMTP id smtpd.web11.1165.1685641681702756065 for ; Thu, 01 Jun 2023 10:48:01 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="signature has expired" header.i=@gmail.com header.s=20221208 header.b=CWneGmfp; spf=pass (domain: gmail.com, ip: 209.85.210.172, mailfrom: kuqin12@gmail.com) Received: by mail-pf1-f172.google.com with SMTP id d2e1a72fcca58-64d30ab1f89so838753b3a.3 for ; Thu, 01 Jun 2023 10:48:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1685641680; x=1688233680; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=fTF7hZwDuMFjezQcxmXAb9pNQK7Yipy2JwA2Be854dY=; b=CWneGmfpUk6H3j2yofGWzRqDimZ0jkAZ04CWbxEon/1oxZd+tdOnCG/qXp9P3KF2gl CvUPZM1BpeKgEFccBHnWI73NOjLbWnIuUi+mtmJgVzqc8pAwyAGswKeWGixbGFJ3vBYY Z/jnQd98wAjNwbLXpF+BLG6NFoA0bMykHtTRyAr/FKG+ynvdUVm16Bj0elGmXt1CFv4t AgKhOF70qTmhVyWTBhhejHrg0gUp2fI1P9nCUECJoWd/pPvz8oFdAru/73KYXB6KdHGI QLriT5WVChIkveGpqIcVCHJWfKWOf5pZyQOqlDY/19h8fj4aHrMkB1ZZib3lobwSe/hh GRhw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1685641680; x=1688233680; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=fTF7hZwDuMFjezQcxmXAb9pNQK7Yipy2JwA2Be854dY=; b=gWY4oHgnkF5UYPyzYy3BYtANiPJSQN5PvKuj4+53nnefFbbq832DWEgzIVJVAF+gNm mhvBkVNiQLZWxf4yfrlvwIaecAO8t97rikr1ldfQ8zyTySg7z2pbMIDRtcdiuhwoYFNh 3hyJn+d9CglEPVxtBc/GpjXTBmC9xbKjsX2yBdZGSjlFHVXjCH/x3qFcDDKlNBKRyufQ 34FJ1Z5gXeajLekidnTeTy5Fpyw07n/TI4J15kXrv3aCALiWrATjtKvr6fey2HbgiqXx xaRL5WA7Bfcx4pb2lQTJb25q7HxHZGlTUY0xvNuAy0eqiEqYbhd2/xi7acNLIEefinEu cfYg== X-Gm-Message-State: AC+VfDy0e5/MtPjQZjylbJkSx2IGGhfNO4kNMOSJYEe0DYawBx83mOBq AELEpyx7/ltbwHz6LqlnlFJ63A+5Rj8= X-Google-Smtp-Source: ACHHUZ4+1IhPeYVJRafPw2MXfCekaTgt56qWRd2A2OgVL+kgAnm5LXn346LDQMM/k/LDgAHVcvjUnQ== X-Received: by 2002:a05:6a00:1816:b0:63b:8423:9e31 with SMTP id y22-20020a056a00181600b0063b84239e31mr8509431pfa.11.1685641680472; Thu, 01 Jun 2023 10:48:00 -0700 (PDT) Return-Path: Received: from ?IPV6:2001:4898:d8:33:e541:1aba:f961:866? ([2001:4898:80e8:f:6555:1aba:f961:866]) by smtp.gmail.com with ESMTPSA id d22-20020aa78e56000000b0064f3bde4981sm5427992pfr.34.2023.06.01.10.47.59 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 01 Jun 2023 10:47:59 -0700 (PDT) Message-ID: <358ac7dc-af06-b167-c465-845e623656c3@gmail.com> Date: Thu, 1 Jun 2023 10:47:59 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.11.0 Subject: Re: [edk2-devel] [PATCH v1 1/1] UnitTestFrameworkPkg: UnitTestPersistenceLib: Save Unit Test Cache to Drive To: devel@edk2.groups.io, michael.d.kinney@intel.com Cc: Sean Brogan , Michael Kubacki References: <20230531191358.587-1-kuqin12@gmail.com> <20230531191358.587-2-kuqin12@gmail.com> <6093b465-6d20-1ab7-6b84-fba55c3fb0df@gmail.com> From: "Kun Qin" In-Reply-To: Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Thanks for the suggestion, Mike. I will try and update the patch per your suggestion. Regards, Kun On 6/1/2023 9:48 AM, Michael D Kinney wrote: > I think your use case is limited to the UnitTestLib based tests running from the UEFI Shell. > > In that case, the library UnitTestFrameworkPkg\Library\UnitTestLib\UnitTestLib.inf is always > linked to the unit test application. You could add a CONSTRUCTOR to UnitTestLib.inf and the > CONSTRUCTOR function can look for UEFI Shell arguments and if args specify alternate path > for unit test cache then save some state for that path. > > Mike > > >> -----Original Message----- >> From: Kun Qin >> Sent: Thursday, June 1, 2023 8:50 AM >> To: Kinney, Michael D ; devel@edk2.groups.io >> Cc: Sean Brogan ; Michael Kubacki >> >> Subject: Re: [PATCH v1 1/1] UnitTestFrameworkPkg: UnitTestPersistenceLib: >> Save Unit Test Cache to Drive >> >> Hi Mike, >> >> Yes, that would indeed serve the purpose. Could you please show me a >> pointer on how to add that >> commonly? I would be open to change the code the other way. >> >> The reason I would like to change this commonly is because if the input >> argument has to be added >> per test, that will prevent the existing tests to be used in the >> aforementioned scenarios. Please let >> me how you think. >> >> Thanks in advance. >> >> Regards, >> Kun >> >> On 5/31/2023 1:39 PM, Kinney, Michael D wrote: >>> Would it be more flexible for unit tests apps to support an optional >> command line arg >>> to specify the volume to write results? >>> >>> Mike >>> >>>> -----Original Message----- >>>> From: Kun Qin >>>> Sent: Wednesday, May 31, 2023 12:14 PM >>>> To: devel@edk2.groups.io >>>> Cc: Sean Brogan ; Michael Kubacki >>>> ; Kinney, Michael D >>>> >>>> Subject: [PATCH v1 1/1] UnitTestFrameworkPkg: UnitTestPersistenceLib: >> Save >>>> Unit Test Cache to Drive >>>> >>>> From: kuqin12 <42554914+kuqin12@users.noreply.github.com> >>>> >>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4467 >>>> >>>> Current implementation of UnitTestFrameworkPkg for shell-based unit test >>>> will save the unit test cache to the same volume as the test application >>>> itself. This works as long as the test application is on a writable >>>> volume, such as USB or EFI partition. >>>> >>>> Instead of saving the files to the same file system of unit test >>>> application, this change will save the cache file to the path where the >>>> user ran this test application. >>>> >>>> This change was tested on proprietary physical hardware platforms and >>>> QEMU based virtual platform. >>>> >>>> Cc: Sean Brogan >>>> Cc: Michael Kubacki >>>> Cc: Michael D Kinney >>>> Signed-off-by: Kun Qin >>>> --- >>>> >>>> >> UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitTes >>>> tPersistenceLibSimpleFileSystem.c | 81 +++++++++++--------- >>>> 1 file changed, 43 insertions(+), 38 deletions(-) >>>> >>>> diff --git >>>> >> a/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitT >>>> estPersistenceLibSimpleFileSystem.c >>>> >> b/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitT >>>> estPersistenceLibSimpleFileSystem.c >>>> index b59991683f48..d4181808b2be 100644 >>>> --- >>>> >> a/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitT >>>> estPersistenceLibSimpleFileSystem.c >>>> +++ >>>> >> b/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitT >>>> estPersistenceLibSimpleFileSystem.c >>>> @@ -22,7 +22,7 @@ >>>> #define CACHE_FILE_SUFFIX L"_Cache.dat" >>>> >>>> >>>> >>>> /** >>>> >>>> - Generate the device path to the cache file. >>>> >>>> + Generate the file name and path to the cache file. >>>> >>>> >>>> >>>> @param[in] FrameworkHandle A pointer to the framework that is >> being >>>> persisted. >>>> >>>> >>>> >>>> @@ -31,8 +31,8 @@ >>>> >>>> >>>> **/ >>>> >>>> STATIC >>>> >>>> -EFI_DEVICE_PATH_PROTOCOL * >>>> >>>> -GetCacheFileDevicePath ( >>>> >>>> +CHAR16 * >>>> >>>> +GetCacheFileName ( >>>> >>>> IN UNIT_TEST_FRAMEWORK_HANDLE FrameworkHandle >>>> >>>> ) >>>> >>>> { >>>> >>>> @@ -85,7 +85,12 @@ GetCacheFileDevicePath ( >>>> // PathCleanUpDirectories (FileNameCopy); >>>> >>>> // if (PathRemoveLastItem (FileNameCopy)) { >>>> >>>> // >>>> >>>> - AppPath = ConvertDevicePathToText (LoadedImage- >>> FilePath, >>>> TRUE, TRUE); // NOTE: This must be freed. >>>> >>>> + AppPath = ConvertDevicePathToText (LoadedImage->FilePath, TRUE, >> TRUE); >>>> // NOTE: This must be freed. >>>> >>>> + if (AppPath == NULL) { >>>> >>>> + DEBUG ((DEBUG_ERROR, "%a - Failed to convert device path to >> text.\n", >>>> __func__)); >>>> >>>> + goto Exit; >>>> >>>> + } >>>> >>>> + >>>> >>>> DirectorySlashOffset = StrLen (AppPath); >>>> >>>> // >>>> >>>> // Make sure we didn't get any weird data. >>>> >>>> @@ -148,15 +153,15 @@ Exit: >>>> FreePool (AppPath); >>>> >>>> } >>>> >>>> >>>> >>>> - if (CacheFilePath != NULL) { >>>> >>>> - FreePool (CacheFilePath); >>>> >>>> + if (CacheFileDevicePath != NULL) { >>>> >>>> + FreePool (CacheFileDevicePath); >>>> >>>> } >>>> >>>> >>>> >>>> if (TestName != NULL) { >>>> >>>> FreePool (TestName); >>>> >>>> } >>>> >>>> >>>> >>>> - return CacheFileDevicePath; >>>> >>>> + return CacheFilePath; >>>> >>>> } >>>> >>>> >>>> >>>> /** >>>> >>>> @@ -175,21 +180,21 @@ DoesCacheExist ( >>>> IN UNIT_TEST_FRAMEWORK_HANDLE FrameworkHandle >>>> >>>> ) >>>> >>>> { >>>> >>>> - EFI_DEVICE_PATH_PROTOCOL *FileDevicePath; >>>> >>>> - EFI_STATUS Status; >>>> >>>> - SHELL_FILE_HANDLE FileHandle; >>>> >>>> + CHAR16 *FileName; >>>> >>>> + EFI_STATUS Status; >>>> >>>> + SHELL_FILE_HANDLE FileHandle; >>>> >>>> >>>> >>>> // >>>> >>>> // NOTE: This devpath is allocated and must be freed. >>>> >>>> // >>>> >>>> - FileDevicePath = GetCacheFileDevicePath (FrameworkHandle); >>>> >>>> + FileName = GetCacheFileName (FrameworkHandle); >>>> >>>> >>>> >>>> // >>>> >>>> // Check to see whether the file exists. If the file can be opened >> for >>>> // reading, it exists. Otherwise, probably not. >>>> >>>> // >>>> >>>> - Status = ShellOpenFileByDevicePath ( >>>> >>>> - &FileDevicePath, >>>> >>>> + Status = ShellOpenFileByName ( >>>> >>>> + FileName, >>>> >>>> &FileHandle, >>>> >>>> EFI_FILE_MODE_READ, >>>> >>>> 0 >>>> >>>> @@ -198,8 +203,8 @@ DoesCacheExist ( >>>> ShellCloseFile (&FileHandle); >>>> >>>> } >>>> >>>> >>>> >>>> - if (FileDevicePath != NULL) { >>>> >>>> - FreePool (FileDevicePath); >>>> >>>> + if (FileName != NULL) { >>>> >>>> + FreePool (FileName); >>>> >>>> } >>>> >>>> >>>> >>>> DEBUG ((DEBUG_VERBOSE, "%a - Returning %d\n", __func__, !EFI_ERROR >>>> (Status))); >>>> >>>> @@ -229,10 +234,10 @@ SaveUnitTestCache ( >>>> IN UINTN SaveStateSize >>>> >>>> ) >>>> >>>> { >>>> >>>> - EFI_DEVICE_PATH_PROTOCOL *FileDevicePath; >>>> >>>> - EFI_STATUS Status; >>>> >>>> - SHELL_FILE_HANDLE FileHandle; >>>> >>>> - UINTN WriteCount; >>>> >>>> + CHAR16 *FileName; >>>> >>>> + EFI_STATUS Status; >>>> >>>> + SHELL_FILE_HANDLE FileHandle; >>>> >>>> + UINTN WriteCount; >>>> >>>> >>>> >>>> // >>>> >>>> // Check the inputs for sanity. >>>> >>>> @@ -245,13 +250,13 @@ SaveUnitTestCache ( >>>> // Determine the path for the cache file. >>>> >>>> // NOTE: This devpath is allocated and must be freed. >>>> >>>> // >>>> >>>> - FileDevicePath = GetCacheFileDevicePath (FrameworkHandle); >>>> >>>> + FileName = GetCacheFileName (FrameworkHandle); >>>> >>>> >>>> >>>> // >>>> >>>> // First lets open the file if it exists so we can delete it...This >> is >>>> the work around for truncation >>>> >>>> // >>>> >>>> - Status = ShellOpenFileByDevicePath ( >>>> >>>> - &FileDevicePath, >>>> >>>> + Status = ShellOpenFileByName ( >>>> >>>> + FileName, >>>> >>>> &FileHandle, >>>> >>>> (EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE), >>>> >>>> 0 >>>> >>>> @@ -270,8 +275,8 @@ SaveUnitTestCache ( >>>> // >>>> >>>> // Now that we know the path to the file... let's open it for >> writing. >>>> // >>>> >>>> - Status = ShellOpenFileByDevicePath ( >>>> >>>> - &FileDevicePath, >>>> >>>> + Status = ShellOpenFileByName ( >>>> >>>> + FileName, >>>> >>>> &FileHandle, >>>> >>>> (EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE | >>>> EFI_FILE_MODE_CREATE), >>>> >>>> 0 >>>> >>>> @@ -304,8 +309,8 @@ SaveUnitTestCache ( >>>> ShellCloseFile (&FileHandle); >>>> >>>> >>>> >>>> Exit: >>>> >>>> - if (FileDevicePath != NULL) { >>>> >>>> - FreePool (FileDevicePath); >>>> >>>> + if (FileName != NULL) { >>>> >>>> + FreePool (FileName); >>>> >>>> } >>>> >>>> >>>> >>>> return Status; >>>> >>>> @@ -334,13 +339,13 @@ LoadUnitTestCache ( >>>> OUT UINTN *SaveStateSize >>>> >>>> ) >>>> >>>> { >>>> >>>> - EFI_STATUS Status; >>>> >>>> - EFI_DEVICE_PATH_PROTOCOL *FileDevicePath; >>>> >>>> - SHELL_FILE_HANDLE FileHandle; >>>> >>>> - BOOLEAN IsFileOpened; >>>> >>>> - UINT64 LargeFileSize; >>>> >>>> - UINTN FileSize; >>>> >>>> - VOID *Buffer; >>>> >>>> + EFI_STATUS Status; >>>> >>>> + CHAR16 *FileName; >>>> >>>> + SHELL_FILE_HANDLE FileHandle; >>>> >>>> + BOOLEAN IsFileOpened; >>>> >>>> + UINT64 LargeFileSize; >>>> >>>> + UINTN FileSize; >>>> >>>> + VOID *Buffer; >>>> >>>> >>>> >>>> IsFileOpened = FALSE; >>>> >>>> Buffer = NULL; >>>> >>>> @@ -356,13 +361,13 @@ LoadUnitTestCache ( >>>> // Determine the path for the cache file. >>>> >>>> // NOTE: This devpath is allocated and must be freed. >>>> >>>> // >>>> >>>> - FileDevicePath = GetCacheFileDevicePath (FrameworkHandle); >>>> >>>> + FileName = GetCacheFileName (FrameworkHandle); >>>> >>>> >>>> >>>> // >>>> >>>> // Now that we know the path to the file... let's open it for >> writing. >>>> // >>>> >>>> - Status = ShellOpenFileByDevicePath ( >>>> >>>> - &FileDevicePath, >>>> >>>> + Status = ShellOpenFileByName ( >>>> >>>> + FileName, >>>> >>>> &FileHandle, >>>> >>>> EFI_FILE_MODE_READ, >>>> >>>> 0 >>>> >>>> @@ -407,8 +412,8 @@ Exit: >>>> // >>>> >>>> // Free allocated buffers >>>> >>>> // >>>> >>>> - if (FileDevicePath != NULL) { >>>> >>>> - FreePool (FileDevicePath); >>>> >>>> + if (FileName != NULL) { >>>> >>>> + FreePool (FileName); >>>> >>>> } >>>> >>>> >>>> >>>> if (IsFileOpened) { >>>> >>>> -- >>>> 2.40.1.windows.1 > > > >