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

Fri Sep 9 10:51:20 PDT 2016

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>