diff mbox series

selftests: user: remove user suite

Message ID 20240725110817.659099-1-usama.anjum@collabora.com
State Accepted
Commit 4e51e13bd986cab31dfab5c3bff1678171debbd1
Headers show
Series selftests: user: remove user suite | expand

Commit Message

Muhammad Usama Anjum July 25, 2024, 11:08 a.m. UTC 
The user test suite has only one test, test_user_copy which loads
test_user_copy module for testing. But test_user_copy module has already
been converted to kunit (see fixes). Hence remove the entire suite.

Fixes: cf6219ee889f ("usercopy: Convert test_user_copy to KUnit test")
Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
 tools/testing/selftests/Makefile               |  1 -
 tools/testing/selftests/user/Makefile          |  9 ---------
 tools/testing/selftests/user/config            |  1 -
 tools/testing/selftests/user/test_user_copy.sh | 18 ------------------
 4 files changed, 29 deletions(-)
 delete mode 100644 tools/testing/selftests/user/Makefile
 delete mode 100644 tools/testing/selftests/user/config
 delete mode 100755 tools/testing/selftests/user/test_user_copy.sh

Comments

Muhammad Usama Anjum July 26, 2024, 8:16 a.m. UTC | #1 
On 7/25/24 7:44 PM, Shuah Khan wrote:
> On 7/25/24 05:08, Muhammad Usama Anjum wrote:
>> The user test suite has only one test, test_user_copy which loads
>> test_user_copy module for testing. But test_user_copy module has already
>> been converted to kunit (see fixes). Hence remove the entire suite.
>>
>> Fixes: cf6219ee889f ("usercopy: Convert test_user_copy to KUnit test")
> 
> Remove fixes tag - this isn't a fix and we don't want this propagating
> to stable releases without kunit test for this.
The user test suite has been failing since cf6219ee889f as the test module
wasn't found. So this is fixing the failure of kselftest. It just causes
noise and may mask other failures. If you still think that fixes isn't
needed, I can send a new version by removing the fixes tag.

> 
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>> ---
> 
> Thanks,
> -- Shuah
> 
>
Shuah Khan July 26, 2024, 5:19 p.m. UTC | #2 
On 7/26/24 02:16, Muhammad Usama Anjum wrote:
> On 7/25/24 7:44 PM, Shuah Khan wrote:
>> On 7/25/24 05:08, Muhammad Usama Anjum wrote:
>>> The user test suite has only one test, test_user_copy which loads
>>> test_user_copy module for testing. But test_user_copy module has already
>>> been converted to kunit (see fixes). Hence remove the entire suite.
>>>
>>> Fixes: cf6219ee889f ("usercopy: Convert test_user_copy to KUnit test")
>>
>> Remove fixes tag - this isn't a fix and we don't want this propagating
>> to stable releases without kunit test for this.
> The user test suite has been failing since cf6219ee889f as the test module
> wasn't found. So this is fixing the failure of kselftest. It just causes
> noise and may mask other failures. If you still think that fixes isn't
> needed, I can send a new version by removing the fixes tag.
> 

In which case this information should have been part of the change log to
make it clear this is a fix. Fixes tag should also mention the releases
this is applicable to so this patch doesn't make it to stables releases
without cf6219ee889f - so this test still runs.

If you are adding Fixes tag it should mention the releases this is
applicable to. Can you get me that information?

thanks,
-- Shuah
Muhammad Usama Anjum July 29, 2024, 7:18 a.m. UTC | #3 
On 7/26/24 10:19 PM, Shuah Khan wrote:
> On 7/26/24 02:16, Muhammad Usama Anjum wrote:
>> On 7/25/24 7:44 PM, Shuah Khan wrote:
>>> On 7/25/24 05:08, Muhammad Usama Anjum wrote:
>>>> The user test suite has only one test, test_user_copy which loads
>>>> test_user_copy module for testing. But test_user_copy module has already
>>>> been converted to kunit (see fixes). Hence remove the entire suite.
>>>>
>>>> Fixes: cf6219ee889f ("usercopy: Convert test_user_copy to KUnit test")
>>>
>>> Remove fixes tag - this isn't a fix and we don't want this propagating
>>> to stable releases without kunit test for this.
>> The user test suite has been failing since cf6219ee889f as the test module
>> wasn't found. So this is fixing the failure of kselftest. It just causes
>> noise and may mask other failures. If you still think that fixes isn't
>> needed, I can send a new version by removing the fixes tag.
>>
> 
> In which case this information should have been part of the change log to
> make it clear this is a fix.
Sorry, I should have mentioned explicitly. Please let me know if I should
send another version for this and the other patch for acceptance [1]?

> Fixes tag should also mention the releases
> this is applicable to so this patch doesn't make it to stables releases
> without cf6219ee889f - so this test still runs.
Release tag is unnecessary as the fix can be applied easily to fixes
commit. From stable-kernel-rules:

  Note, such tagging is unnecessary if the stable team can derive the
  appropriate versions from Fixes: tags.

> 
> If you are adding Fixes tag it should mention the releases this is
> applicable to. Can you get me that information?
> 
> thanks,
> -- Shuah
> 
> 

[1]
https://lore.kernel.org/all/20240725121212.808206-1-usama.anjum@collabora.com
Shuah Khan July 30, 2024, 10:36 p.m. UTC | #4 
On 7/25/24 08:44, Shuah Khan wrote:
> On 7/25/24 05:08, Muhammad Usama Anjum wrote:
>> The user test suite has only one test, test_user_copy which loads
>> test_user_copy module for testing. But test_user_copy module has already
>> been converted to kunit (see fixes). Hence remove the entire suite.
>>
>> Fixes: cf6219ee889f ("usercopy: Convert test_user_copy to KUnit test")
> 
> Remove fixes tag - this isn't a fix and we don't want this propagating
> to stable releases without kunit test for this.
> 
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>> ---
> 
> Thanks,
> -- Shuah
> 

As mentioned in other threads on this conversion to kunit and removal
of kselfttest - NACK on this patch.

Please don't send me any more of these conversion and removal patches.

thanks,
-- Shuah
Kees Cook July 31, 2024, 2:33 a.m. UTC | #5 
On July 30, 2024 3:36:11 PM PDT, Shuah Khan <skhan@linuxfoundation.org> wrote:
>On 7/25/24 08:44, Shuah Khan wrote:
>> On 7/25/24 05:08, Muhammad Usama Anjum wrote:
>>> The user test suite has only one test, test_user_copy which loads
>>> test_user_copy module for testing. But test_user_copy module has already
>>> been converted to kunit (see fixes). Hence remove the entire suite.
>>> 
>>> Fixes: cf6219ee889f ("usercopy: Convert test_user_copy to KUnit test")
>> 
>> Remove fixes tag - this isn't a fix and we don't want this propagating
>> to stable releases without kunit test for this.
>> 
>>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>>> ---
>> 
>> Thanks,
>> -- Shuah
>> 
>
>As mentioned in other threads on this conversion to kunit and removal
>of kselfttest - NACK on this patch.
>
>Please don't send me any more of these conversion and removal patches.
>

I think there is a misunderstanding about these particular patches (for string and usercopy selftests). Those were already converted, as desired, by the maintainer (me) to KUnit. These associated patches are cleaning up the dangling kselftest part of them, and should land (with the Fixes tag, which is aimed at the commit that did the conversion).

-Kees
diff mbox series

Patch

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index bc8fe9e8f7f20..af2429431b6b2 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -107,7 +107,6 @@  TARGETS += tmpfs
 TARGETS += tpm2
 TARGETS += tty
 TARGETS += uevent
-TARGETS += user
 TARGETS += user_events
 TARGETS += vDSO
 TARGETS += mm
diff --git a/tools/testing/selftests/user/Makefile b/tools/testing/selftests/user/Makefile
deleted file mode 100644
index 640a40f9b72bc..0000000000000
--- a/tools/testing/selftests/user/Makefile
+++ /dev/null
@@ -1,9 +0,0 @@ 
-# SPDX-License-Identifier: GPL-2.0-only
-# Makefile for user memory selftests
-
-# No binaries, but make sure arg-less "make" doesn't trigger "run_tests"
-all:
-
-TEST_PROGS := test_user_copy.sh
-
-include ../lib.mk
diff --git a/tools/testing/selftests/user/config b/tools/testing/selftests/user/config
deleted file mode 100644
index 784ed8416324d..0000000000000
--- a/tools/testing/selftests/user/config
+++ /dev/null
@@ -1 +0,0 @@ 
-CONFIG_TEST_USER_COPY=m
diff --git a/tools/testing/selftests/user/test_user_copy.sh b/tools/testing/selftests/user/test_user_copy.sh
deleted file mode 100755
index f9b31a57439b7..0000000000000
--- a/tools/testing/selftests/user/test_user_copy.sh
+++ /dev/null
@@ -1,18 +0,0 @@ 
-#!/bin/sh
-# SPDX-License-Identifier: GPL-2.0
-# Runs copy_to/from_user infrastructure using test_user_copy kernel module
-
-# Kselftest framework requirement - SKIP code is 4.
-ksft_skip=4
-
-if ! /sbin/modprobe -q -n test_user_copy; then
-	echo "user: module test_user_copy is not found [SKIP]"
-	exit $ksft_skip
-fi
-if /sbin/modprobe -q test_user_copy; then
-	/sbin/modprobe -q -r test_user_copy
-	echo "user_copy: ok"
-else
-	echo "user_copy: [FAIL]"
-	exit 1
-fi