| Message ID | 20190424231237.14776-1-keescook@chromium.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | selftests: Move test output to diagnostic lines | expand |
On 4/24/19 5:12 PM, wrote: > This refactors the selftest Makefiles to extract the test running logic > to be reused between "run_tests" and "emit_tests", while also fixing > up the test output to be TAP version 13 compliant: > - added "plan" line > - fixed result line syntax > - moved all test output to be "# "-prefixed as TAP "diagnostic" lines > > The prefixing code includes a fallback mode for limited execution > environments. > > Additionally, the plan lines are fixed for all callers of kselftest.h. > > -Kees > Kees, Just about to apply these to a topic branch to do testing and ran into checkpatch errors: WARNING: line over 80 characters - a few WARNING: Misplaced SPDX-License-Identifier tag - use line 1 instead #141: FILE: tools/testing/selftests/kselftest/runner.sh:2: Can fix them and resend - SPDX one is my main concern. The plan is to apply these to linux-kselftest ksft-tap-refactor topic first. I don't want to rush these until we do some testing. thanks, -- Shuah
On Thu, Apr 25, 2019 at 9:52 AM shuah <shuah@kernel.org> wrote: > > On 4/24/19 5:12 PM, wrote: > > This refactors the selftest Makefiles to extract the test running logic > > to be reused between "run_tests" and "emit_tests", while also fixing > > up the test output to be TAP version 13 compliant: > > - added "plan" line > > - fixed result line syntax > > - moved all test output to be "# "-prefixed as TAP "diagnostic" lines > > > > The prefixing code includes a fallback mode for limited execution > > environments. > > > > Additionally, the plan lines are fixed for all callers of kselftest.h. > > > > -Kees > > > > Kees, > > Just about to apply these to a topic branch to do testing and ran into > checkpatch errors: > > > WARNING: line over 80 characters - a few I only saw one, which is on a string which kernel coding style says to leave unsplit: WARNING: line over 80 characters #55: FILE: tools/testing/selftests/kselftest/runner.sh:19: + echo "$TEST_HDR_MSG: Warning: file $TEST is not executable, correct this." > WARNING: Misplaced SPDX-License-Identifier tag - use line 1 instead > #141: FILE: tools/testing/selftests/kselftest/runner.sh:2: WARNING: Misplaced SPDX-License-Identifier tag - use line 1 instead #37: FILE: tools/testing/selftests/kselftest/runner.sh:2: # SPDX-License-Identifier: GPL-2.0 This is a shell script. It can't be on line 1: $ head -n3 tools/testing/selftests/kselftest/runner.sh #!/bin/sh # SPDX-License-Identifier: GPL-2.0 # That looks like a bug in checkpatch not resetting the expected line or something. > Can fix them and resend - SPDX one is my main concern. These appear to be false positives; I don't think I need to fix them? Let me know what you think. > The plan is to apply these to linux-kselftest ksft-tap-refactor topic > first. I don't want to rush these until we do some testing. Absolutely. :) Thanks!
On Thu, Apr 25, 2019 at 10:05 AM Kees Cook <keescook@chromium.org> wrote: > WARNING: Misplaced SPDX-License-Identifier tag - use line 1 instead > #37: FILE: tools/testing/selftests/kselftest/runner.sh:2: > # SPDX-License-Identifier: GPL-2.0 > > This is a shell script. It can't be on line 1: > > $ head -n3 tools/testing/selftests/kselftest/runner.sh > #!/bin/sh > # SPDX-License-Identifier: GPL-2.0 > # > > That looks like a bug in checkpatch not resetting the expected line or > something. It doesn't like patch 3 and doesn't notice that diff offset starts at line 2: diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh index e1117d703887..f12b0a631273 100644 --- a/tools/testing/selftests/kselftest/runner.sh +++ b/tools/testing/selftests/kselftest/runner.sh @@ -2,17 +2,20 @@ # SPDX-License-Identifier: GPL-2.0 # # Runs a set of tests in a given subdirectory. +export KSFT_TAP_LEVEL=1 Joe, looks like the problem is here: if ($realline == $checklicenseline) { realline == 2, checklicenseline == 1 so this is skipped, including the "#!/" check to move checklicenseline to 2. then: if ($realline != $checklicenseline && $rawline =~ /\bSPDX-License-Identifier:/ && realline == 2, checklicenseline == 1 throws warning. Seems like checklicenseline should be unconditionally set to 2 for ".sh" files? I don't see a way to fix this for just missing the #!/ line from a context diff, though...
On 4/25/19 11:05 AM, Kees Cook wrote: > On Thu, Apr 25, 2019 at 9:52 AM shuah <shuah@kernel.org> wrote: >> >> On 4/24/19 5:12 PM, wrote: >>> This refactors the selftest Makefiles to extract the test running logic >>> to be reused between "run_tests" and "emit_tests", while also fixing >>> up the test output to be TAP version 13 compliant: >>> - added "plan" line >>> - fixed result line syntax >>> - moved all test output to be "# "-prefixed as TAP "diagnostic" lines >>> >>> The prefixing code includes a fallback mode for limited execution >>> environments. >>> >>> Additionally, the plan lines are fixed for all callers of kselftest.h. >>> >>> -Kees >>> >> >> Kees, >> >> Just about to apply these to a topic branch to do testing and ran into >> checkpatch errors: >> >> >> WARNING: line over 80 characters - a few > > I only saw one, which is on a string which kernel coding style says to > leave unsplit: > > WARNING: line over 80 characters > #55: FILE: tools/testing/selftests/kselftest/runner.sh:19: > + echo "$TEST_HDR_MSG: Warning: file $TEST is not > executable, correct this." > >> WARNING: Misplaced SPDX-License-Identifier tag - use line 1 instead >> #141: FILE: tools/testing/selftests/kselftest/runner.sh:2: > > WARNING: Misplaced SPDX-License-Identifier tag - use line 1 instead > #37: FILE: tools/testing/selftests/kselftest/runner.sh:2: > # SPDX-License-Identifier: GPL-2.0 > > This is a shell script. It can't be on line 1: > > $ head -n3 tools/testing/selftests/kselftest/runner.sh > #!/bin/sh > # SPDX-License-Identifier: GPL-2.0 > # > > That looks like a bug in checkpatch not resetting the expected line or > something. > >> Can fix them and resend - SPDX one is my main concern. > > These appear to be false positives; I don't think I need to fix them? > Let me know what you think. > >> The plan is to apply these to linux-kselftest ksft-tap-refactor topic >> first. I don't want to rush these until we do some testing. > > Absolutely. :) > > Thanks! > Kees, Pushed all 8 patches to https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest ksft-tap-refactor topic branch I will have time to test tomorrow. thanks, -- Shuah
On Thu, Apr 25, 2019 at 1:39 PM shuah <shuah@kernel.org> wrote: > Pushed all 8 patches to > https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest > ksft-tap-refactor topic branch > > I will have time to test tomorrow. Awesome! I'm excited. :) Next I want to update kselftest_harness.h to do header/plan printing...