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

Fri Sep 9 11:44:01 PDT 2016

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
> 