[edk2-devel] [PATCH v1 1/1] UnitTestFrameworkPkg: UnitTestPersistenceLib: Save Unit Test Cache to Drive
Kun Qin
kuqin12 at gmail.com
Thu Jun 1 17:47:59 UTC 2023
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 <kuqin12 at gmail.com>
>> Sent: Thursday, June 1, 2023 8:50 AM
>> To: Kinney, Michael D <michael.d.kinney at intel.com>; devel at edk2.groups.io
>> Cc: Sean Brogan <sean.brogan at microsoft.com>; Michael Kubacki
>> <mikuback at linux.microsoft.com>
>> 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 <kuqin12 at gmail.com>
>>>> Sent: Wednesday, May 31, 2023 12:14 PM
>>>> To: devel at edk2.groups.io
>>>> Cc: Sean Brogan <sean.brogan at microsoft.com>; Michael Kubacki
>>>> <mikuback at linux.microsoft.com>; Kinney, Michael D
>>>> <michael.d.kinney at intel.com>
>>>> Subject: [PATCH v1 1/1] UnitTestFrameworkPkg: UnitTestPersistenceLib:
>> Save
>>>> Unit Test Cache to Drive
>>>>
>>>> From: kuqin12 <42554914+kuqin12 at 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 <sean.brogan at microsoft.com>
>>>> Cc: Michael Kubacki <mikuback at linux.microsoft.com>
>>>> Cc: Michael D Kinney <michael.d.kinney at intel.com>
>>>> Signed-off-by: Kun Qin <kuqin12 at gmail.com>
>>>> ---
>>>>
>>>>
>> 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
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105596): https://edk2.groups.io/g/devel/message/105596
Mute This Topic: https://groups.io/mt/99249446/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list