From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f173.google.com (mail-pl1-f173.google.com [209.85.214.173]) by mx.groups.io with SMTP id smtpd.web11.36752.1685634616928063467 for ; Thu, 01 Jun 2023 08:50:16 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20221208 header.b=jag84qvy; spf=pass (domain: gmail.com, ip: 209.85.214.173, mailfrom: kuqin12@gmail.com) Received: by mail-pl1-f173.google.com with SMTP id d9443c01a7336-1b0478487c3so4841855ad.1 for ; Thu, 01 Jun 2023 08:50:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1685634616; x=1688226616; 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=Kt7inj0n7TIkl6UFzevxcTIfXDKH86/3Vk1EfHzRnUs=; b=jag84qvycUnaEazbA8LhSx8FLIGMobnY37FEOcO89AP138/CGAs5aOtVt485ocvEW2 rz1RCYdDYOFShVrWfcZuWdgZmkAW9PUkmwjUCh+OYpOdySzp1FfYH8FUCT3+DDuXz57n aqCtFhjzk/6itVl/SFudxbgFlEw5K1U7WMrBgEOUsHw2CHs81MDJhTx1PzQ2+8MRKXtC RB2EQKODqEjhUO/jw3XMNGrg0KDBPzdWZ8BCmeW7SK7buBljvDdWmPviRhE48cE27VcB NFB0bFT7Hug1rtN8Yl+0PxZXbji3sQTquzRgkCzo3a81r/1dnjXnqTXTlxITmWxfuae4 LU9w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1685634616; x=1688226616; 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=Kt7inj0n7TIkl6UFzevxcTIfXDKH86/3Vk1EfHzRnUs=; b=CGaT+7Qn2Wcf9tXy6Oi4xJvJJzE+i8NHgRp+C4Ip6NBd0/8276UYUNz5xMNYyYZHMr 3HbZRvopYmfJ/GW2KwQzwn/w0aybshR2oVWcnpnonCycbpfQmi1BT+DDRwwDR2Zw2DF/ ybm7ze46EB/9jaTfcGQJoReLnLR45K7Xbkf98AtjBjeeNCOVlnRvVe5Gq5iCnNs24jeo IMn87Usn0GTzjs8Mptfs88d2IpeZv0btfi+tU3kMjIMlPKyGfMltPz3MQGdbXOiaDf8H J8+cli6GGFj/8q1Gb96B1St/XaX/016ieHhVr0zD1Z7b5UmgkKWjXW99hvdhIlVr0M9t QjnA== X-Gm-Message-State: AC+VfDyoVRsYwVKMz85a2+h0QYCKsyLCb/vmtFqzMrHDDgpkEqdMb08f HnvscL6j5vi19WTBCX4ERSk= X-Google-Smtp-Source: ACHHUZ57limWmGClwL6zmGyrhIZJ/KDHSfV/D/ypOMvM+GUA1rCeIzd/EIcsqBDhc+FQF5IomIazTQ== X-Received: by 2002:a17:902:e848:b0:1b1:8aa1:3a48 with SMTP id t8-20020a170902e84800b001b18aa13a48mr2826226plg.42.1685634616122; Thu, 01 Jun 2023 08:50:16 -0700 (PDT) Return-Path: Received: from ?IPV6:2001:4898:d8:33:e541:1aba:f961:866? ([2001:4898:80e8:36:652e:1aba:f961:866]) by smtp.gmail.com with ESMTPSA id n6-20020a170902d2c600b001a4fecf79e4sm3690783plc.49.2023.06.01.08.50.15 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 01 Jun 2023 08:50:15 -0700 (PDT) Message-ID: <6093b465-6d20-1ab7-6b84-fba55c3fb0df@gmail.com> Date: Thu, 1 Jun 2023 08:50:13 -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: [PATCH v1 1/1] UnitTestFrameworkPkg: UnitTestPersistenceLib: Save Unit Test Cache to Drive To: "Kinney, Michael D" , "devel@edk2.groups.io" Cc: Sean Brogan , Michael Kubacki References: <20230531191358.587-1-kuqin12@gmail.com> <20230531191358.587-2-kuqin12@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 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