[llvm] r281008 - [llvm-cov] Speculate fix for a Windows-only test (NFC)

Fri Sep 9 13:31:13 PDT 2016

Thanks again for the help.

The bot is green now:

  http://lab.llvm.org:8011/builders/clang-x86-windows-msvc2015/builds/230

> On Sep 9, 2016, at 11:44 AM, Vedant Kumar <vsk at apple.com> wrote:
> 
> Sorry for missing this. I gave your suggestion a shot (r281072), and I'll be
> watching this bot.
> 
> thanks,
> vedant
> 
>> On Sep 9, 2016, at 11:28 AM, Reid Kleckner <rnk at google.com> wrote:
>> 
>> If you look at the test output, you can see there are other issues here:
>> http://lab.llvm.org:8011/builders/clang-x86-windows-msvc2015/builds/227/steps/ninja%20check%201/logs/FAIL%3A%20LLVM%3A%3Anative_separators.c
>> 
>> $ ...
>> # command stderr:
>> error: /tmp/native_separators.c: no such file or directory
>> warning: The file '/tmp/native_separators.c' isn't covered.
>> 
>> $ ...
>> # command stderr:
>> error: ../C:/b/slave/clang-x86-windows-msvc2015/clang-x86-windows-msvc2015/stage1/./bin\llvm-config.EXE/../C:/b/slave/clang-x86-windows-msvc2015/clang-x86-windows-msvc2015/stage1/./bin\llvm-cov.EXE/native_separators.c: invalid argument
>> warning: The file '/tmp/native_separators.c' isn't covered.
>> 
>> I think you should add quotes into the llvm-config and llvm-cov parts of that command to avoid the expansion from the directory name to the path to the tool, like /llvm-"config"/ and /llvm-"cov"/. The shell will interpret those strings correctly.
>> 
>> On Fri, Sep 9, 2016 at 10:51 AM, Vedant Kumar via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>> Thanks Maggie! The patch looks good, committed as r281062.
>> 
>> Yaron, you're seeing the assertion failure because the html coverage renderer
>> expects the covmapping file to be younger than the source files it displays.
>> The covmapping file encodes source region information which the renderer
>> depends on. When you changed the length of the "TEXT-INDEX" line, the renderer
>> got upset and claimed it couldn't figure out how to display that line.
>> 
>> Maybe it's time to remove the asserts... In Release mode, the renderer behaves
>> gracefully when these assertions fail.
>> 
>> vedant
>> 
>>> On Sep 9, 2016, at 4:43 AM, Ying Yi <maggieyi666 at gmail.com> wrote:
>>> 
>>> Hi,
>>> 
>>> Since the test only runs on Windows machine and the native separator on Windows is ‘\’, we shouldn’t change the test. I attached a patch that fixes the issue.
>>> 
>>> @Vedant, could you please review the patch? Please feel free to commit the patch if you are happy with it.
>>> 
>>> Thanks,
>>> 
>>> On Fri, Sep 9, 2016 at 10:46 AM, Yaron Keren <yaron.keren at gmail.com> wrote:
>>> Hi,
>>> 
>>> this still fails due to wrong path seperator. I tried to apply the usual fix:
>>> 
>>> // TEXT-INDEX: {{/|\\}}tmp{{/|\\}}native_separators.c
>>> 
>>> but then an assert fails
>>> 
>>> 
>>> $ "C:/llvm-clean/msvc/RelWithDebInfo/bin\FileCheck.EXE" "-check-prefixes=TEXT-INDEX" "-input-file=C:\llvm-clean\msvc\test\tools\llvm-cov\Output\native_separators.c.tmp.dir/index.txt" "C:\llvm-clean\test\tools\llvm-cov\native_separators.c" $ "C:/llvm-clean/msvc/RelWithDebInfo/bin\llvm-cov.EXE" "show" "-format=html" "C:\llvm-clean\test\tools\llvm-cov/Inputs/native_separators.covmapping" "-instr-profile=C:\llvm-clean\msvc\test\tools\llvm-cov\Output\native_separators.c.tmp.profdata" "-filename-equivalence" "../C:/llvm-clean/msvc/RelWithDebInfo/bin\llvm-config.EXE/../C:/llvm-clean/msvc/RelWithDebInfo/bin\llvm-cov.EXE/native_separators.c" "-o" "C:\llvm-clean\msvc\test\tools\llvm-cov\Output\native_separators.c.tmp.dir"
>>> # command stderr:
>>> error: ../C:/llvm-clean/msvc/RelWithDebInfo/bin\llvm-config.EXE/../C:/llvm-clean/msvc/RelWithDebInfo/bin\llvm-cov.EXE/native_separators.c: invalid argument
>>> warning: The file '/tmp/native_separators.c' isn't covered.
>>> 
>>> $ "C:/llvm-clean/msvc/RelWithDebInfo/bin\FileCheck.EXE" "-check-prefixes=HTML-INDEX" "-input-file=C:\llvm-clean\msvc\test\tools\llvm-cov\Output\native_separators.c.tmp.dir/index.html" "C:\llvm-clean\test\tools\llvm-cov\native_separators.c"
>>> 
>>> $ "C:/llvm-clean/msvc/RelWithDebInfo/bin\llvm-cov.EXE" "show" "-format=html" "C:\llvm-clean\test\tools\llvm-cov/Inputs/native_separators.covmapping" "-instr-profile=C:\llvm-clean\msvc\test\tools\llvm-cov\Output\native_separators.c.tmp.profdata" "-filename-equivalence" "C:\llvm-clean\test\tools\llvm-cov\native_separators.c" "-o" "C:\llvm-clean\msvc\test\tools\llvm-cov\Output\native_separators.c.tmp.dir"
>>> # command stderr:
>>> Assertion failed: Start + Len <= Line.size() && "Snippet extends past the EOL", file C:\llvm-clean\tools\llvm-cov\SourceCoverageViewHTML.cpp, line 476
>>> 
>>> 
>>> 
>>> as Ying wrote, if the line
>>> 
>>> // RUN: FileCheck -check-prefixes=TEXT-INDEX -input-file=%t.dir/index.txt %s
>>> 
>>> is removed the test passes.
>>> 
>>> Yaron
>>> 
>>> 
>>> 
>>> 2016-09-09 12:43 GMT+03:00 Ying Yi via llvm-commits <llvm-commits at lists.llvm.org>:
>>> Hi Vedant,
>>> 
>>> 
>>> 
>>>> Could you help test this out? I don't have a Windows machine available.
>>> 
>>> Of course. The test still fails but for a different reason. It currently fails when running the following test:
>>> 
>>> 
>>> 
>>> // RUN: llvm-cov show %S/Inputs/native_separators.covmapping -instr-profile=%t.profdata -o %t.dir
>>> 
>>> // RUN: FileCheck -check-prefixes=TEXT-INDEX -input-file=%t.dir/index.txt %s
>>> 
>>> // TEXT-INDEX: \tmp\native_separators.c
>>> 
>>> 
>>> 
>>> The test expects that the native separators are used in the source files list in both text and html index pages.
>>> 
>>> With the latest llvm-cov, only the source file list in the index.html file uses the native separators. The index.txt file does not use the native separators in the source file list.
>>> 
>>> 
>>> 
>>> If I remove the single test above, the rest of the native_separators.c tests pass.
>>> 
>>> 
>>> 
>>> Please let me know if you need any further information.
>>> 
>>> 
>>> 
>>> Regard,
>>> 
>>> Maggie
>>> 
>>> 
>>> On Fri, Sep 9, 2016 at 2:43 AM, Vedant Kumar <vsk at apple.com> wrote:
>>> Hi Maggie,
>>> 
>>> Sorry for breaking this test!
>>> 
>>> Could you help test this out? I don't have a Windows machine available.
>>> 
>>> thanks!
>>> vedant
>>> 
>>>> On Sep 8, 2016, at 6:32 PM, Vedant Kumar via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>>>> 
>>>> Author: vedantk
>>>> Date: Thu Sep  8 20:32:47 2016
>>>> New Revision: 281008
>>>> 
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=281008&view=rev
>>>> Log:
>>>> [llvm-cov] Speculate fix for a Windows-only test (NFC)
>>>> 
>>>> This test should have broken after r280896. Fix up the test case
>>>> speculatively, since I don't have a way to test it.
>>>> 
>>>> I wonder why I didn't get any angry bot emails about this. Maybe none of
>>>> the win32 bots test llvm-cov? That could explain it, since the test says
>>>> it 'REQUIRES: system-windows', which is restricted to win32 hosts.
>>>> 
>>>> Also: why is 'system-windows' not defined for non-win32 Windows bots?
>>>> 
>>>> Modified:
>>>>   llvm/trunk/test/tools/llvm-cov/native_separators.c
>>>> 
>>>> Modified: llvm/trunk/test/tools/llvm-cov/native_separators.c
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-cov/native_separators.c?rev=281008&r1=281007&r2=281008&view=diff
>>>> ==============================================================================
>>>> --- llvm/trunk/test/tools/llvm-cov/native_separators.c (original)
>>>> +++ llvm/trunk/test/tools/llvm-cov/native_separators.c Thu Sep  8 20:32:47 2016
>>>> @@ -15,15 +15,13 @@
>>>> 
>>>> // TEXT-INDEX: \tmp\native_separators.c
>>>> // HTML-INDEX: >tmp\native_separators.c</a>
>>>> -// HTML: <pre>Source: \tmp\native_separators.c</pre>
>>>> -// HTML: tools\llvm-cov\Inputs\native_separators.covmapping</pre>
>>>> +// HTML: <pre>Source: \tmp\native_separators.c (Binary: native_separators.covmapping)</pre>
>>>> 
>>>> int main() {}
>>>> 
>>>> // RUN: llvm-cov show %S/Inputs/native_separators.covmapping -instr-profile=%t.profdata -filename-equivalence %s -o %t.dir
>>>> // RUN: FileCheck -check-prefixes=TEXT -input-file=%t.dir/coverage/tmp/native_separators.c.txt %s
>>>> -// TEXT: {{^}}Source: \tmp\native_separators.c:{{$}}
>>>> -// TEXT: {{^}}Binary: {{.*}}tools\llvm-cov\Inputs\native_separators.covmapping:{{$}}
>>>> +// TEXT: Source: \tmp\native_separators.c (Binary: native_separators.covmapping)
>>>> 
>>>> // Re-purpose this file to test that "Go to first unexecuted line" feature.
>>>> 
>>>> 
>>>> 
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>> 
>>> 
>>> 
>>> 
>>> --
>>> Ying Yi
>>> SN Systems - Sony Interactive Entertainment
>>> 
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>> 
>>> 
>>> 
>>> 
>>> 
>>> --
>>> Ying Yi
>>> SN Systems - Sony Interactive Entertainment
>>> <native.patch>
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>> 
> 