[RESEND PATCH v3] list: test: Mending tests for list_cut_position()

[RESEND PATCH v3] list: test: Mending tests for list_cut_position()

[I Hsin Cheng]

Mending test for list_cut_position*() for the missing check of integer
"i" after the second loop. The variable should be checked for second
time to make sure both lists after the cut operation are formed as
expected.

Signed-off-by: I Hsin Cheng <richard120310@gmail.com>
---
 lib/list-test.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/list-test.c b/lib/list-test.c
index 37cbc33e9fdb..b4b3810c71d0 100644
--- a/lib/list-test.c
+++ b/lib/list-test.c
@@ -408,6 +408,8 @@ static void list_test_list_cut_position(struct kunit *test)
 		KUNIT_EXPECT_PTR_EQ(test, cur, &entries[i]);
 		i++;
 	}
+
+	KUNIT_EXPECT_EQ(test, i, 3);
 }
 
 static void list_test_list_cut_before(struct kunit *test)
@@ -436,6 +438,8 @@ static void list_test_list_cut_before(struct kunit *test)
 		KUNIT_EXPECT_PTR_EQ(test, cur, &entries[i]);
 		i++;
 	}
+
+	KUNIT_EXPECT_EQ(test, i, 3);
 }
 
 static void list_test_list_splice(struct kunit *test)
-- 
2.43.0

Re: [RESEND PATCH v3] list: test: Mending tests for list_cut_position()

[David Gow]

[-- Attachment #1: Type: text/plain, Size: 2242 bytes --]

On Tue, 1 Oct 2024 at 01:06, I Hsin Cheng <richard120310@gmail.com> wrote:
>
> Mending test for list_cut_position*() for the missing check of integer
> "i" after the second loop. The variable should be checked for second
> time to make sure both lists after the cut operation are formed as
> expected.
>
> Signed-off-by: I Hsin Cheng <richard120310@gmail.com>
> ---

Thanks. I've tested this, and it works now and makes sense.

I would recommend updating the patch description slightly, as it's a
little bit confusing as-is (partly due to the early version having
already been applied and reverted).
Could we describe this (a) in the imperative mood, and (b) focus less
on this being a "fix" or "mend" and more on what the new check does.
For example, something like:
"Check the total number of elements in both resultant lists are
correct. Previously, only the first list's size was checked, so
additional elements in the second list would not have been caught."

Otherwise, this is fine.

Reviewed-by: David Gow <davidgow@google.com>

Thanks for your patience,
-- David




>  lib/list-test.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/lib/list-test.c b/lib/list-test.c
> index 37cbc33e9fdb..b4b3810c71d0 100644
> --- a/lib/list-test.c
> +++ b/lib/list-test.c
> @@ -408,6 +408,8 @@ static void list_test_list_cut_position(struct kunit *test)
>                 KUNIT_EXPECT_PTR_EQ(test, cur, &entries[i]);
>                 i++;
>         }
> +
> +       KUNIT_EXPECT_EQ(test, i, 3);
>  }
>
>  static void list_test_list_cut_before(struct kunit *test)
> @@ -436,6 +438,8 @@ static void list_test_list_cut_before(struct kunit *test)
>                 KUNIT_EXPECT_PTR_EQ(test, cur, &entries[i]);
>                 i++;
>         }
> +
> +       KUNIT_EXPECT_EQ(test, i, 3);
>  }
>
>  static void list_test_list_splice(struct kunit *test)
> --
> 2.43.0
>
> --
> You received this message because you are subscribed to the Google Groups "KUnit Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20240930170633.42475-1-richard120310%40gmail.com.

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5294 bytes --]

Re: [RESEND PATCH v3] list: test: Mending tests for list_cut_position()

[I Hsin Cheng]

Thu, Oct 03, 2024 at 02:49:15PM +0800, David Gow wrote:
> On Tue, 1 Oct 2024 at 01:06, I Hsin Cheng <richard120310@gmail.com> wrote:
> >
> > Mending test for list_cut_position*() for the missing check of integer
> > "i" after the second loop. The variable should be checked for second
> > time to make sure both lists after the cut operation are formed as
> > expected.
> >
> > Signed-off-by: I Hsin Cheng <richard120310@gmail.com>
> > ---
> 
> Thanks. I've tested this, and it works now and makes sense.
> 
> I would recommend updating the patch description slightly, as it's a
> little bit confusing as-is (partly due to the early version having
> already been applied and reverted).
> Could we describe this (a) in the imperative mood, and (b) focus less
> on this being a "fix" or "mend" and more on what the new check does.
> For example, something like:
> "Check the total number of elements in both resultant lists are
> correct. Previously, only the first list's size was checked, so
> additional elements in the second list would not have been caught."
> 
> Otherwise, this is fine.
> 
> Reviewed-by: David Gow <davidgow@google.com>
> 
> Thanks for your patience,
> -- David
> 
> 
> 
> 
> >  lib/list-test.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/lib/list-test.c b/lib/list-test.c
> > index 37cbc33e9fdb..b4b3810c71d0 100644
> > --- a/lib/list-test.c
> > +++ b/lib/list-test.c
> > @@ -408,6 +408,8 @@ static void list_test_list_cut_position(struct kunit *test)
> >                 KUNIT_EXPECT_PTR_EQ(test, cur, &entries[i]);
> >                 i++;
> >         }
> > +
> > +       KUNIT_EXPECT_EQ(test, i, 3);
> >  }
> >
> >  static void list_test_list_cut_before(struct kunit *test)
> > @@ -436,6 +438,8 @@ static void list_test_list_cut_before(struct kunit *test)
> >                 KUNIT_EXPECT_PTR_EQ(test, cur, &entries[i]);
> >                 i++;
> >         }
> > +
> > +       KUNIT_EXPECT_EQ(test, i, 3);
> >  }
> >
> >  static void list_test_list_splice(struct kunit *test)
> > --
> > 2.43.0
> >
> > --
> > You received this message because you are subscribed to the Google Groups "KUnit Development" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20240930170633.42475-1-richard120310%40gmail.com.

Hello Mr. Gow, thanks for your review and sorry for the late reply.

> I would recommend updating the patch description slightly, as it's a
> little bit confusing as-is (partly due to the early version having
> already been applied and reverted).

No problem, I'll refine the commit message, hoever, I want to ask do I
have to send a new patch for this ( since the commit title and commit
message will be different ) , or do I just resent the patch with RESEND
v4 in this thread?

Best regards,
Richard Cheng.

Re: [RESEND PATCH v3] list: test: Mending tests for list_cut_position()

[Andrew Morton]

On Tue, 8 Oct 2024 00:20:43 +0800 I Hsin Cheng <richard120310@gmail.com> wrote:

> > I would recommend updating the patch description slightly, as it's a
> > little bit confusing as-is (partly due to the early version having
> > already been applied and reverted).
> 
> No problem, I'll refine the commit message, hoever, I want to ask do I
> have to send a new patch for this ( since the commit title and commit
> message will be different ) , or do I just resent the patch with RESEND
> v4 in this thread?

Please just send the complete v4.