diff mbox

[5/5,Resend] uefirtvariable: Add new test for UEFI runtime SetVariable, subtest 5

Message ID 1354705707-9595-1-git-send-email-ivan.hu@canonical.com
State Accepted
Headers show

Commit Message

Ivan Hu Dec. 5, 2012, 11:08 a.m. UTC 
This test tests the UEFI runtime service SetVariable interface.
SetVariable when the Attributes is 0. The expected return status
is EFI_NOT_FOUND.

Signed-off-by: Ivan Hu <ivan.hu@canonical.com>
---
 src/uefi/uefirtvariable/uefirtvariable.c |   24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

Comments

Colin Ian King Dec. 5, 2012, 11:36 a.m. UTC | #1 
On 05/12/12 11:08, Ivan Hu wrote:
> This test tests the UEFI runtime service SetVariable interface.
> SetVariable when the Attributes is 0. The expected return status
> is EFI_NOT_FOUND.
>
> Signed-off-by: Ivan Hu <ivan.hu@canonical.com>
> ---
>   src/uefi/uefirtvariable/uefirtvariable.c |   24 ++++++++++++++++++++++++
>   1 file changed, 24 insertions(+)
>
> diff --git a/src/uefi/uefirtvariable/uefirtvariable.c b/src/uefi/uefirtvariable/uefirtvariable.c
> index 3c50059..1599c5b 100644
> --- a/src/uefi/uefirtvariable/uefirtvariable.c
> +++ b/src/uefi/uefirtvariable/uefirtvariable.c
> @@ -556,6 +556,25 @@ static int setvariable_test4(fwts_framework *fw, uint32_t attributes)
>   	return FWTS_OK;
>   }
>
> +static int setvariable_test5(fwts_framework *fw, uint32_t attributes)
> +{
> +	uint64_t datasize = 10;
> +	uint8_t datadiff = 0;
> +
> +	if (setvariable_insertvariable(fw, attributes, datasize, variablenametest,
> +							&gtestguid1, datadiff) == FWTS_ERROR)
> +		return FWTS_ERROR;
> +
> +	if (setvariable_insertvariable(fw, 0, datasize, variablenametest,
> +							&gtestguid1, datadiff) == FWTS_ERROR)
> +		return FWTS_ERROR;
> +
> +	if (setvariable_checkvariable_notfound(fw, variablenametest, &gtestguid1) == FWTS_ERROR)
> +		return FWTS_ERROR;
> +
> +	return FWTS_OK;
> +}
> +
>   static int uefirtvariable_test1(fwts_framework *fw)
>   {
>   	uint64_t index;
> @@ -611,6 +630,11 @@ static int uefirtvariable_test3(fwts_framework *fw)
>   			return FWTS_ERROR;
>   	}
>
> +	for (index = 0; index < (sizeof(attributesarray)/(sizeof attributesarray[0])); index++) {
> +		if (setvariable_test5(fw, attributesarray[index]) == FWTS_ERROR)
> +			return FWTS_ERROR;
> +	}
> +
>   	fwts_passed(fw, "UEFI runtime service SetVariable interface test passed.");
>
>   	return FWTS_OK;
>
Acked-by: Colin King <colin.king@canonical.com>
Colin Ian King Dec. 5, 2012, 11:57 a.m. UTC | #2 
Thanks Ivan for the UEFI runtime SetVariable tests.

I've ACK'd these tests as they exercise the SetVariable runtime well 
enough.  I suspect we may need to re-visit these at a later date and 
perhaps had some more feedback to the user in the event of failures.

For example, setvariable_insertvariable reports back:

"Failed to set variable with UEFI runtime service."

..which is useful to know the that something failed, but I think we 
could expand this a little more. For example, maybe we could add some 
more feedback to the user such as the length of the variable name, size 
of the data, it's attributes, etc.   Imagine if you are running the test 
and you get a failed error - what does that relate to? As a user, it 
could perhaps be useful to know.

Also, let's look at patch 2, uefirtvariable_test1(), you added:


+    for (index = 0; index < (sizeof(attributesarray)/(sizeof 
attributesarray[0])); index++) {
+        if (setvariable_test2(fw, attributesarray[index], 
variablenametest) == FWTS_ERROR)
+            return FWTS_ERROR;
+    }

Note that one can add multiple fwts_passed() and fwts_failed() messages 
in the test, so for each of these minor tests you could feedback a 
fwts_passed() message..

e.g.

	fwts_log_info(fw, "Testing SetVariable on a variable with two different 
sizes.");
         .. do the setvariable_test2() testss..
	for (index = 0; index < ... etc) {
		if (setvariable_test2(fw, ...) == FWTS_ERROR)
			return FWTS_FAILED;

         fwts_passed(fw, "SetVariable on a variable with two different 
sizes passed.");

Anyhow, I'm OK with the tests as they are, but perhaps we should 
re-visit them and add a little more feedback if it doesn't clutter up 
the output too much.

Colin
Ivan Hu Dec. 7, 2012, 10:21 a.m. UTC | #3 
Hi Coling,

Thanks for your valuable suggestion.
I'll try to add another patch for these useful failed information.
Let me know if you have any suggestions in that coming patch. Thanks!

Cheers,
Ivan

On 12/05/2012 07:57 PM, Colin Ian King wrote:
> Thanks Ivan for the UEFI runtime SetVariable tests.
>
> I've ACK'd these tests as they exercise the SetVariable runtime well
> enough.  I suspect we may need to re-visit these at a later date and
> perhaps had some more feedback to the user in the event of failures.
>
> For example, setvariable_insertvariable reports back:
>
> "Failed to set variable with UEFI runtime service."
>
> ..which is useful to know the that something failed, but I think we
> could expand this a little more. For example, maybe we could add some
> more feedback to the user such as the length of the variable name, size
> of the data, it's attributes, etc.   Imagine if you are running the test
> and you get a failed error - what does that relate to? As a user, it
> could perhaps be useful to know.
>
> Also, let's look at patch 2, uefirtvariable_test1(), you added:
>
>
> +    for (index = 0; index < (sizeof(attributesarray)/(sizeof
> attributesarray[0])); index++) {
> +        if (setvariable_test2(fw, attributesarray[index],
> variablenametest) == FWTS_ERROR)
> +            return FWTS_ERROR;
> +    }
>
> Note that one can add multiple fwts_passed() and fwts_failed() messages
> in the test, so for each of these minor tests you could feedback a
> fwts_passed() message..
>
> e.g.
>
>      fwts_log_info(fw, "Testing SetVariable on a variable with two
> different sizes.");
>          .. do the setvariable_test2() testss..
>      for (index = 0; index < ... etc) {
>          if (setvariable_test2(fw, ...) == FWTS_ERROR)
>              return FWTS_FAILED;
>
>          fwts_passed(fw, "SetVariable on a variable with two different
> sizes passed.");
>
> Anyhow, I'm OK with the tests as they are, but perhaps we should
> re-visit them and add a little more feedback if it doesn't clutter up
> the output too much.
>
> Colin
>
Keng-Yu Lin Dec. 10, 2012, 8:02 a.m. UTC | #4 
On Wed, Dec 5, 2012 at 7:08 PM, Ivan Hu <ivan.hu@canonical.com> wrote:
> This test tests the UEFI runtime service SetVariable interface.
> SetVariable when the Attributes is 0. The expected return status
> is EFI_NOT_FOUND.
>
> Signed-off-by: Ivan Hu <ivan.hu@canonical.com>
> ---
>  src/uefi/uefirtvariable/uefirtvariable.c |   24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>
> diff --git a/src/uefi/uefirtvariable/uefirtvariable.c b/src/uefi/uefirtvariable/uefirtvariable.c
> index 3c50059..1599c5b 100644
> --- a/src/uefi/uefirtvariable/uefirtvariable.c
> +++ b/src/uefi/uefirtvariable/uefirtvariable.c
> @@ -556,6 +556,25 @@ static int setvariable_test4(fwts_framework *fw, uint32_t attributes)
>         return FWTS_OK;
>  }
>
> +static int setvariable_test5(fwts_framework *fw, uint32_t attributes)
> +{
> +       uint64_t datasize = 10;
> +       uint8_t datadiff = 0;
> +
> +       if (setvariable_insertvariable(fw, attributes, datasize, variablenametest,
> +                                                       &gtestguid1, datadiff) == FWTS_ERROR)
> +               return FWTS_ERROR;
> +
> +       if (setvariable_insertvariable(fw, 0, datasize, variablenametest,
> +                                                       &gtestguid1, datadiff) == FWTS_ERROR)
> +               return FWTS_ERROR;
> +
> +       if (setvariable_checkvariable_notfound(fw, variablenametest, &gtestguid1) == FWTS_ERROR)
> +               return FWTS_ERROR;
> +
> +       return FWTS_OK;
> +}
> +
>  static int uefirtvariable_test1(fwts_framework *fw)
>  {
>         uint64_t index;
> @@ -611,6 +630,11 @@ static int uefirtvariable_test3(fwts_framework *fw)
>                         return FWTS_ERROR;
>         }
>
> +       for (index = 0; index < (sizeof(attributesarray)/(sizeof attributesarray[0])); index++) {
> +               if (setvariable_test5(fw, attributesarray[index]) == FWTS_ERROR)
> +                       return FWTS_ERROR;
> +       }
> +
>         fwts_passed(fw, "UEFI runtime service SetVariable interface test passed.");
>
>         return FWTS_OK;
> --
> 1.7.10.4
>
Acked-by: Keng-Yu Lin <kengyu@canonical.com>
diff mbox

Patch

diff --git a/src/uefi/uefirtvariable/uefirtvariable.c b/src/uefi/uefirtvariable/uefirtvariable.c
index 3c50059..1599c5b 100644
--- a/src/uefi/uefirtvariable/uefirtvariable.c
+++ b/src/uefi/uefirtvariable/uefirtvariable.c
@@ -556,6 +556,25 @@  static int setvariable_test4(fwts_framework *fw, uint32_t attributes)
 	return FWTS_OK;
 }
 
+static int setvariable_test5(fwts_framework *fw, uint32_t attributes)
+{
+	uint64_t datasize = 10;
+	uint8_t datadiff = 0;
+
+	if (setvariable_insertvariable(fw, attributes, datasize, variablenametest,
+							&gtestguid1, datadiff) == FWTS_ERROR)
+		return FWTS_ERROR;
+
+	if (setvariable_insertvariable(fw, 0, datasize, variablenametest,
+							&gtestguid1, datadiff) == FWTS_ERROR)
+		return FWTS_ERROR;
+
+	if (setvariable_checkvariable_notfound(fw, variablenametest, &gtestguid1) == FWTS_ERROR)
+		return FWTS_ERROR;
+
+	return FWTS_OK;
+}
+
 static int uefirtvariable_test1(fwts_framework *fw)
 {
 	uint64_t index;
@@ -611,6 +630,11 @@  static int uefirtvariable_test3(fwts_framework *fw)
 			return FWTS_ERROR;
 	}
 
+	for (index = 0; index < (sizeof(attributesarray)/(sizeof attributesarray[0])); index++) {
+		if (setvariable_test5(fw, attributesarray[index]) == FWTS_ERROR)
+			return FWTS_ERROR;
+	}
+
 	fwts_passed(fw, "UEFI runtime service SetVariable interface test passed.");
 
 	return FWTS_OK;