When generating the selftests to another folder, the fixed tests are missing as they are not in Makefile. The missing tests are generated by command: $ for f in $(ls *.sh); do grep -q $f Makefile || echo $f; done
I think there need a way to notify the developer when they created a new file in selftests folder. Maybe a bot like bluez.test.bot or kernel test robot could help do that?
Hangbin Liu (2): selftests/net: add missing tests selftests/net/forwarding: add missing tests
tools/testing/selftests/net/Makefile | 3 +- .../testing/selftests/net/forwarding/Makefile | 33 +++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-)
When generating the selftests to another folder, the fixed tests are missing as they are not in Makefile, e.g.
make -C tools/testing/selftests/ install \ TARGETS="net" INSTALL_PATH=/tmp/kselftests
Signed-off-by: Hangbin Liu liuhangbin@gmail.com --- tools/testing/selftests/net/Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile index 3fe2515aa616..0f2ebc38d893 100644 --- a/tools/testing/selftests/net/Makefile +++ b/tools/testing/selftests/net/Makefile @@ -30,7 +30,7 @@ TEST_PROGS += ioam6.sh TEST_PROGS += gro.sh TEST_PROGS += gre_gso.sh TEST_PROGS += cmsg_so_mark.sh -TEST_PROGS += cmsg_time.sh +TEST_PROGS += cmsg_time.sh cmsg_ipv6.sh TEST_PROGS += srv6_end_dt46_l3vpn_test.sh TEST_PROGS += srv6_end_dt4_l3vpn_test.sh TEST_PROGS += srv6_end_dt6_l3vpn_test.sh @@ -54,6 +54,7 @@ TEST_GEN_PROGS = reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa TEST_GEN_PROGS += reuseport_dualstack reuseaddr_conflict tls TEST_GEN_FILES += toeplitz TEST_GEN_FILES += cmsg_sender +TEST_PROGS += test_vxlan_vnifiltering.sh
TEST_FILES := settings
When generating the selftests to another folder, the fixed tests are missing as they are not in Makefile, e.g.
make -C tools/testing/selftests/ install \ TARGETS="net/forwarding" INSTALL_PATH=/tmp/kselftests
Signed-off-by: Hangbin Liu liuhangbin@gmail.com --- .../testing/selftests/net/forwarding/Makefile | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+)
diff --git a/tools/testing/selftests/net/forwarding/Makefile b/tools/testing/selftests/net/forwarding/Makefile index 8fa97ae9af9e..c87e674b61b1 100644 --- a/tools/testing/selftests/net/forwarding/Makefile +++ b/tools/testing/selftests/net/forwarding/Makefile @@ -2,15 +2,31 @@
TEST_PROGS = bridge_igmp.sh \ bridge_locked_port.sh \ + bridge_mld.sh \ bridge_port_isolation.sh \ bridge_sticky_fdb.sh \ bridge_vlan_aware.sh \ + bridge_vlan_mcast.sh \ bridge_vlan_unaware.sh \ + custom_multipath_hash.sh \ + dual_vxlan_bridge.sh \ + ethtool_extended_state.sh \ ethtool.sh \ + gre_custom_multipath_hash.sh \ gre_inner_v4_multipath.sh \ gre_inner_v6_multipath.sh \ + gre_multipath_nh_res.sh \ + gre_multipath_nh.sh \ gre_multipath.sh \ + hw_stats_l3.sh \ ip6_forward_instats_vrf.sh \ + ip6gre_custom_multipath_hash.sh \ + ip6gre_flat_key.sh \ + ip6gre_flat_keys.sh \ + ip6gre_flat.sh \ + ip6gre_hier_key.sh \ + ip6gre_hier_keys.sh \ + ip6gre_hier.sh \ ip6gre_inner_v4_multipath.sh \ ip6gre_inner_v6_multipath.sh \ ipip_flat_gre_key.sh \ @@ -34,36 +50,53 @@ TEST_PROGS = bridge_igmp.sh \ mirror_gre_vlan_bridge_1q.sh \ mirror_gre_vlan.sh \ mirror_vlan.sh \ + pedit_dsfield.sh \ + pedit_ip.sh \ + pedit_l4port.sh \ + q_in_vni_ipv6.sh \ + q_in_vni.sh \ router_bridge.sh \ router_bridge_vlan.sh \ router_broadcast.sh \ + router_mpath_nh_res.sh \ router_mpath_nh.sh \ router_multicast.sh \ router_multipath.sh \ + router_nh.sh \ router.sh \ router_vid_1.sh \ sch_ets.sh \ + sch_red.sh \ sch_tbf_ets.sh \ sch_tbf_prio.sh \ sch_tbf_root.sh \ + skbedit_priority.sh \ tc_actions.sh \ tc_chains.sh \ tc_flower_router.sh \ tc_flower.sh \ tc_mpls_l2vpn.sh \ + tc_police.sh \ tc_shblocks.sh \ tc_vlan_modify.sh \ + vxlan_asymmetric_ipv6.sh \ vxlan_asymmetric.sh \ + vxlan_bridge_1d_ipv6.sh \ + vxlan_bridge_1d_port_8472_ipv6.sh \ vxlan_bridge_1d_port_8472.sh \ vxlan_bridge_1d.sh \ + vxlan_bridge_1q_ipv6.sh \ + vxlan_bridge_1q_port_8472_ipv6.sh vxlan_bridge_1q_port_8472.sh \ vxlan_bridge_1q.sh \ + vxlan_symmetric_ipv6.sh \ vxlan_symmetric.sh
TEST_PROGS_EXTENDED := devlink_lib.sh \ ethtool_lib.sh \ fib_offload_lib.sh \ forwarding.config.sample \ + ip6gre_lib.sh \ ipip_lib.sh \ lib.sh \ mirror_gre_lib.sh \
On Thu, 28 Apr 2022 12:45:09 +0800 Hangbin Liu wrote:
When generating the selftests to another folder, the fixed tests are missing as they are not in Makefile. The missing tests are generated by command: $ for f in $(ls *.sh); do grep -q $f Makefile || echo $f; done
I think there need a way to notify the developer when they created a new file in selftests folder. Maybe a bot like bluez.test.bot or kernel test robot could help do that?
Our netdev patch checks are here:
https://github.com/kuba-moo/nipa/tree/master/tests/patch
in case you're willing to code it up and post a PR.
On Fri, Apr 29, 2022 at 05:56:04PM -0700, Jakub Kicinski wrote:
On Thu, 28 Apr 2022 12:45:09 +0800 Hangbin Liu wrote:
I think there need a way to notify the developer when they created a new file in selftests folder. Maybe a bot like bluez.test.bot or kernel test robot could help do that?
Our netdev patch checks are here:
https://github.com/kuba-moo/nipa/tree/master/tests/patch
in case you're willing to code it up and post a PR.
Hi Jakub,
I checked the tools and write a draft patch. But I have a question before post the PR. AFAIK, This bot is only used for checking patches and adding status in patchwork. But it doesn't support sending a reply to developer, right?
For the selftest reminder, I think it would be good to let developer know via email if the file is missing in Makefile. What do you think?
Here is the draft patch:
diff --git a/tests/patch/check_selftest/check_selftest.sh b/tests/patch/check_selftest/check_selftest.sh new file mode 100755 index 0000000..ad7c608 --- /dev/null +++ b/tests/patch/check_selftest/check_selftest.sh @@ -0,0 +1,28 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 + +rt=0 +if ! git show --name-status --oneline | \ + grep -P '^A\ttools/testing/selftests/net/' | \ + grep '.sh$'; then + echo "No new net selftests script" >&$DESC_FD + exit 0 +fi + +files=$(git show --name-status --oneline | grep -P '^A\ttools/testing/selftests/net/' | grep '.sh$' | sed 's@A\ttools/testing/selftests/net/@@') +for file in $files; do + if echo $file | grep forwarding; then + file=$(echo $file | sed 's/forwarding///') + if ! grep -P "[\t| ]$file" tools/testing/selftests/net/forwarding/Makefile;then + echo "new test $file not in selftests/net/forwarding/Makefile" >&$DESC_FD + rc=1 + fi + else + if ! grep -P "[\t| ]$file" tools/testing/selftests/net/Makefile;then + echo "new test $file not in selftests/net/Makefile" >&$DESC_FD + rc=1 + fi + fi +done + +exit $rc diff --git a/tests/patch/check_selftest/info.json b/tests/patch/check_selftest/info.json new file mode 100644 index 0000000..615779f --- /dev/null +++ b/tests/patch/check_selftest/info.json @@ -0,0 +1,3 @@ +{ + "run": ["check_selftest.sh"] +}
On Tue, 17 May 2022 14:23:22 +0800 Hangbin Liu wrote:
On Fri, Apr 29, 2022 at 05:56:04PM -0700, Jakub Kicinski wrote:
On Thu, 28 Apr 2022 12:45:09 +0800 Hangbin Liu wrote:
I think there need a way to notify the developer when they created a new file in selftests folder. Maybe a bot like bluez.test.bot or kernel test robot could help do that?
Our netdev patch checks are here:
https://github.com/kuba-moo/nipa/tree/master/tests/patch
in case you're willing to code it up and post a PR.
Hi Jakub,
I checked the tools and write a draft patch. But I have a question before post the PR.
First off - thanks a log for doing this!
AFAIK, This bot is only used for checking patches and adding status in patchwork. But it doesn't support sending a reply to developer, right?
For the selftest reminder, I think it would be good to let developer know via email if the file is missing in Makefile. What do you think?
Yes, we don't have the auto-reply. There's too much noise in some of the tests, but mostly it's because we don't want to encourage people posting patches just to build them. If it's a machine replying rather than a human some may think that it's okay. We already have jaw-droppingly expensive VM instance to keep up with the build volume. And the list is very busy. So we can't afford "post to run the CI" development model.
Here is the draft patch:
diff --git a/tests/patch/check_selftest/check_selftest.sh b/tests/patch/check_selftest/check_selftest.sh new file mode 100755 index 0000000..ad7c608 --- /dev/null +++ b/tests/patch/check_selftest/check_selftest.sh @@ -0,0 +1,28 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0
+rt=0 +if ! git show --name-status --oneline | \
- grep -P '^A\ttools/testing/selftests/net/' | \
- grep '.sh$'; then
- echo "No new net selftests script" >&$DESC_FD
- exit 0
+fi
+files=$(git show --name-status --oneline | grep -P '^A\ttools/testing/selftests/net/' | grep '.sh$' | sed 's@A\ttools/testing/selftests/net/@@') +for file in $files; do
- if echo $file | grep forwarding; then
file=$(echo $file | sed 's/forwarding\///')if ! grep -P "[\t| ]$file" tools/testing/selftests/net/forwarding/Makefile;thenecho "new test $file not in selftests/net/forwarding/Makefile" >&$DESC_FDrc=1fi- else
if ! grep -P "[\t| ]$file" tools/testing/selftests/net/Makefile;thenecho "new test $file not in selftests/net/Makefile" >&$DESC_FDrc=1fi
Does it matter which exact selftest makefile the changes are? Maybe as a first stab we should just check if there are changes to anything in tools/testing/selftests/.*/Makefile?
We can see if there are false-negatives.
- fi
+done
+exit $rc diff --git a/tests/patch/check_selftest/info.json b/tests/patch/check_selftest/info.json new file mode 100644 index 0000000..615779f --- /dev/null +++ b/tests/patch/check_selftest/info.json @@ -0,0 +1,3 @@ +{
- "run": ["check_selftest.sh"]
+}
On Tue, May 17, 2022 at 12:45:17PM -0700, Jakub Kicinski wrote:
Yes, we don't have the auto-reply. There's too much noise in some of the tests, but mostly it's because we don't want to encourage people posting patches just to build them. If it's a machine replying rather than a human some may think that it's okay. We already have jaw-droppingly expensive VM instance to keep up with the build volume. And the list is very busy. So we can't afford "post to run the CI" development model.
OK, I just afraid the developer doesn't check patchwork status.
+files=$(git show --name-status --oneline | grep -P '^A\ttools/testing/selftests/net/' | grep '.sh$' | sed 's@A\ttools/testing/selftests/net/@@') +for file in $files; do
- if echo $file | grep forwarding; then
file=$(echo $file | sed 's/forwarding\///')if ! grep -P "[\t| ]$file" tools/testing/selftests/net/forwarding/Makefile;thenecho "new test $file not in selftests/net/forwarding/Makefile" >&$DESC_FDrc=1fi- else
if ! grep -P "[\t| ]$file" tools/testing/selftests/net/Makefile;thenecho "new test $file not in selftests/net/Makefile" >&$DESC_FDrc=1fiDoes it matter which exact selftest makefile the changes are?
I only checked the tools/testing/selftests/net/Makefile and tools/testing/selftests/net/forwarding/Makefile at present. Maybe mptcp should also added?
Maybe as a first stab we should just check if there are changes to anything in tools/testing/selftests/.*/Makefile?
In my checking only shell scripts are checked, as most net net/forwarding tests using shell script for testing. But other sub-component may use c binary or python for testing. So I think there is no need to check all tools/testing/selftests/.*/Makefile. WDYT?
Thanks Hangbin
On Wed, 18 May 2022 14:01:43 +0800 Hangbin Liu wrote:
+files=$(git show --name-status --oneline | grep -P '^A\ttools/testing/selftests/net/' | grep '.sh$' | sed 's@A\ttools/testing/selftests/net/@@')
FWIW this will list just the names of bash scripts with no decoration:
git show --pretty="" --name-only -- tools/testing/selftests/*.sh
And we can get the names of the files with basename:
for f in $(git show --pretty="" --name-only); do basename $f; done
+for file in $files; do
- if echo $file | grep forwarding; then
file=$(echo $file | sed 's/forwarding\///')if ! grep -P "[\t| ]$file" tools/testing/selftests/net/forwarding/Makefile;thenecho "new test $file not in selftests/net/forwarding/Makefile" >&$DESC_FDrc=1fi- else
if ! grep -P "[\t| ]$file" tools/testing/selftests/net/Makefile;thenecho "new test $file not in selftests/net/Makefile" >&$DESC_FDrc=1fiDoes it matter which exact selftest makefile the changes are?
I only checked the tools/testing/selftests/net/Makefile and tools/testing/selftests/net/forwarding/Makefile at present. Maybe mptcp should also added?
Right, mptcp is one example, then we also have tools/testing/selftests/drivers/net/
There may be new directories added, then we'd need to keep updating the test.
Maybe as a first stab we should just check if there are changes to anything in tools/testing/selftests/.*/Makefile?
In my checking only shell scripts are checked, as most net net/forwarding tests using shell script for testing. But other sub-component may use c binary or python for testing. So I think there is no need to check all tools/testing/selftests/.*/Makefile. WDYT?
Not sure I understand, let me explain what I meant in more detail. I think we should make it generic. For example check the Makefile in the same location as the script:
grep $(basename $f) $(dirname $f)/Makefile
And maybe just to be safe one directory level down?
grep $(basename $f) $(dirname $(dirname $f))/Makefile
Instead of hardcoding the expected paths.
On Wed, May 18, 2022 at 08:25:48AM -0700, Jakub Kicinski wrote:
On Wed, 18 May 2022 14:01:43 +0800 Hangbin Liu wrote:
+files=$(git show --name-status --oneline | grep -P '^A\ttools/testing/selftests/net/' | grep '.sh$' | sed 's@A\ttools/testing/selftests/net/@@')
FWIW this will list just the names of bash scripts with no decoration:
git show --pretty="" --name-only -- tools/testing/selftests/*.sh
And we can get the names of the files with basename:
for f in $(git show --pretty="" --name-only); do basename $f; done
This way is easier :)
python for testing. So I think there is no need to check all tools/testing/selftests/.*/Makefile. WDYT?
Not sure I understand, let me explain what I meant in more detail. I think we should make it generic. For example check the Makefile in the same location as the script:
grep $(basename $f) $(dirname $f)/Makefile
And maybe just to be safe one directory level down?
grep $(basename $f) $(dirname $(dirname $f))/Makefile
Instead of hardcoding the expected paths.
Ah, got what you mean. Thanks. I will check how to update the script and open PR after that.
Cheers Hangbin
Hello:
This series was applied to netdev/net.git (master) by Jakub Kicinski kuba@kernel.org:
On Thu, 28 Apr 2022 12:45:09 +0800 you wrote:
When generating the selftests to another folder, the fixed tests are missing as they are not in Makefile. The missing tests are generated by command: $ for f in $(ls *.sh); do grep -q $f Makefile || echo $f; done
I think there need a way to notify the developer when they created a new file in selftests folder. Maybe a bot like bluez.test.bot or kernel test robot could help do that?
[...]
Here is the summary with links: - [net,1/2] selftests/net: add missing tests to Makefile https://git.kernel.org/netdev/net/c/38dcd9570d6f - [net,2/2] selftests/net/forwarding: add missing tests to Makefile https://git.kernel.org/netdev/net/c/f62c5acc800e
You are awesome, thank you!
linux-kselftest-mirror@lists.linaro.org
-
Hangbin Liu -
Jakub Kicinski -
patchwork-bot+netdevbpf＠kernel.org