On Tue, 4 Mar 2025 16:22:43 GMT, SendaoYan <s...@openjdk.org> wrote:
> Hi all,
>
> This PR fix the makefile bug when there is no gtest 'disable tests' then
> report syntax error.
>
> Before this PR makefile used below command to get the 'disable tests' number,
> when there is no 'YOU HAVE [0-9]+ DISABLED TEST' string line in gtest result
> file gtest.txt, this gawk command will not print anything, so shell report
> syntax error later.
>
>
> gawk '/YOU HAVE [0-9]+ DISABLED TEST/ { if (match($0, /[0-9]+/, arr)) { print
> arr[0]; found=1; } if (!found) { print 0; } }'
> build/linux-x86_64-server-release/test-results/gtest_Align_server/gtest.txt
>
>
> After this PR makefile will use below command to get the 'disable tests'
> number.
>
>
> gawk '/YOU HAVE [0-9]+ DISABLED TEST/ { if (match($0, /YOU HAVE ([0-9]+)
> DISABLED TEST/, arr)) { print arr[1]; found=1; } } END { if (!found) print 0;
> }' build/linux-x86_64-server-release/test-results/gtest_Align_server/gtest.txt
>
>
> Additional testing:
>
> - [x] make test TEST=gtest:Align > test-Alian.log, which return 0 disable test
> - [x] time make test TEST=gtest > test-all.log, which return 15 disable tests
>
> [test-all.log](https://github.com/user-attachments/files/19074190/test-all.log)
> [test-Alian.log](https://github.com/user-attachments/files/19074189/test-Alian.log)
I haven't tested my suggestion here, but this based on how I understand awk.
make/RunTests.gmk line 543:
> 541: '/YOU HAVE [0-9]+ DISABLED TEST/ { \
> 542: if (match($$$$0, /YOU HAVE ([0-9]+) DISABLED TEST/, arr)) { \
> 543: print arr[1]; \
I can't see how this change is needed. The block after the `/.../` regex will
only be executed for lines matching the regex.
make/RunTests.gmk line 546:
> 544: found=1; \
> 545: } } END \
> 546: { if (!found) print 0; }' \
>From what I can tell, this is the relevant fix, moving the fallback 0 to the
>`END` block. I would suggest reformatting this for better readability.
>Probably something like this. The `END` keyword should be at the same level as
>the regex line matcher above, though the mix of indentation levels here is a
>bit confusing.
Suggestion:
} \
} \
END { if (!found) print 0; }' \
-------------
PR Review: https://git.openjdk.org/jdk/pull/23904#pullrequestreview-2658383562
PR Review Comment: https://git.openjdk.org/jdk/pull/23904#discussion_r1979893492
PR Review Comment: https://git.openjdk.org/jdk/pull/23904#discussion_r1979891983
