| Message ID | 20210410175601.831013-1-joe@cilium.io (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next] selftests/bpf: Make docs tests fail more reliably | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for bpf-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | warning | 9 maintainers not CCed: linux-kselftest@vger.kernel.org netdev@vger.kernel.org yhs@fb.com kpsingh@kernel.org andrii@kernel.org kafai@fb.com john.fastabend@gmail.com songliubraving@fb.com shuah@kernel.org |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 15 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
On Sat, Apr 10, 2021 at 10:57 AM Joe Stringer <joe@cilium.io> wrote: > > Previously, if rst2man caught errors, then these would be ignored and > the output file would be written anyway. This would allow developers to > introduce regressions in the docs comments in the BPF headers. > > Additionally, even if you instruct rst2man to fail out, it will still > write out to the destination target file, so if you ran the tests twice > in a row it would always pass. Use a temporary file for the initial run > to ensure that if rst2man fails out under "--strict" mode, subsequent > runs will not automatically pass. > > Tested via ./tools/testing/selftests/bpf/test_doc_build.sh > > Signed-off-by: Joe Stringer <joe@cilium.io> > --- > tools/testing/selftests/bpf/Makefile.docs | 3 ++- > tools/testing/selftests/bpf/test_doc_build.sh | 1 + > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/bpf/Makefile.docs b/tools/testing/selftests/bpf/Makefile.docs > index ccf260021e83..a918790c8f9c 100644 > --- a/tools/testing/selftests/bpf/Makefile.docs > +++ b/tools/testing/selftests/bpf/Makefile.docs > @@ -52,7 +52,8 @@ $(OUTPUT)%.$2: $(OUTPUT)%.rst > ifndef RST2MAN_DEP > $$(error "rst2man not found, but required to generate man pages") > endif > - $$(QUIET_GEN)rst2man $$< > $$@ > + $$(QUIET_GEN)rst2man --strict $$< > $$@.tmp > + $$(QUIET_GEN)mv $$@.tmp $$@ if something goes wrong this .tmp file will be laying around, so we should at least add it to .gitignore? > > docs-clean-$1: > $$(call QUIET_CLEAN, eBPF_$1-manpage) > diff --git a/tools/testing/selftests/bpf/test_doc_build.sh b/tools/testing/selftests/bpf/test_doc_build.sh > index 7eb940a7b2eb..ed12111cd2f0 100755 > --- a/tools/testing/selftests/bpf/test_doc_build.sh > +++ b/tools/testing/selftests/bpf/test_doc_build.sh > @@ -1,5 +1,6 @@ > #!/bin/bash > # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +set -e > > # Assume script is located under tools/testing/selftests/bpf/. We want to start > # build attempts from the top of kernel repository. > -- > 2.27.0 >
On Mon, Apr 12, 2021 at 8:51 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Sat, Apr 10, 2021 at 10:57 AM Joe Stringer <joe@cilium.io> wrote: > > > > Previously, if rst2man caught errors, then these would be ignored and > > the output file would be written anyway. This would allow developers to > > introduce regressions in the docs comments in the BPF headers. > > > > Additionally, even if you instruct rst2man to fail out, it will still > > write out to the destination target file, so if you ran the tests twice > > in a row it would always pass. Use a temporary file for the initial run > > to ensure that if rst2man fails out under "--strict" mode, subsequent > > runs will not automatically pass. > > > > Tested via ./tools/testing/selftests/bpf/test_doc_build.sh > > > > Signed-off-by: Joe Stringer <joe@cilium.io> > > --- > > tools/testing/selftests/bpf/Makefile.docs | 3 ++- > > tools/testing/selftests/bpf/test_doc_build.sh | 1 + > > 2 files changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/tools/testing/selftests/bpf/Makefile.docs b/tools/testing/selftests/bpf/Makefile.docs > > index ccf260021e83..a918790c8f9c 100644 > > --- a/tools/testing/selftests/bpf/Makefile.docs > > +++ b/tools/testing/selftests/bpf/Makefile.docs > > @@ -52,7 +52,8 @@ $(OUTPUT)%.$2: $(OUTPUT)%.rst > > ifndef RST2MAN_DEP > > $$(error "rst2man not found, but required to generate man pages") > > endif > > - $$(QUIET_GEN)rst2man $$< > $$@ > > + $$(QUIET_GEN)rst2man --strict $$< > $$@.tmp > > + $$(QUIET_GEN)mv $$@.tmp $$@ > > if something goes wrong this .tmp file will be laying around, so we > should at least add it to .gitignore? Right, doesn't look like it's there yet. Will fix it. I also received out-of-band feedback to use --exit-status rather than --strict so I'll fix that up as well for a v2. Cheers, Joe
diff --git a/tools/testing/selftests/bpf/Makefile.docs b/tools/testing/selftests/bpf/Makefile.docs index ccf260021e83..a918790c8f9c 100644 --- a/tools/testing/selftests/bpf/Makefile.docs +++ b/tools/testing/selftests/bpf/Makefile.docs @@ -52,7 +52,8 @@ $(OUTPUT)%.$2: $(OUTPUT)%.rst ifndef RST2MAN_DEP $$(error "rst2man not found, but required to generate man pages") endif - $$(QUIET_GEN)rst2man $$< > $$@ + $$(QUIET_GEN)rst2man --strict $$< > $$@.tmp + $$(QUIET_GEN)mv $$@.tmp $$@ docs-clean-$1: $$(call QUIET_CLEAN, eBPF_$1-manpage) diff --git a/tools/testing/selftests/bpf/test_doc_build.sh b/tools/testing/selftests/bpf/test_doc_build.sh index 7eb940a7b2eb..ed12111cd2f0 100755 --- a/tools/testing/selftests/bpf/test_doc_build.sh +++ b/tools/testing/selftests/bpf/test_doc_build.sh @@ -1,5 +1,6 @@ #!/bin/bash # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +set -e # Assume script is located under tools/testing/selftests/bpf/. We want to start # build attempts from the top of kernel repository.
Previously, if rst2man caught errors, then these would be ignored and the output file would be written anyway. This would allow developers to introduce regressions in the docs comments in the BPF headers. Additionally, even if you instruct rst2man to fail out, it will still write out to the destination target file, so if you ran the tests twice in a row it would always pass. Use a temporary file for the initial run to ensure that if rst2man fails out under "--strict" mode, subsequent runs will not automatically pass. Tested via ./tools/testing/selftests/bpf/test_doc_build.sh Signed-off-by: Joe Stringer <joe@cilium.io> --- tools/testing/selftests/bpf/Makefile.docs | 3 ++- tools/testing/selftests/bpf/test_doc_build.sh | 1 + 2 files changed, 3 insertions(+), 1 deletion(-)