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

Fri Sep 9 11:28:46 PDT 2016

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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160909/24016b3a/attachment.html>