[PATCH] Added basic testing support for llvm-cov.

Bob Wilson bob.wilson at apple.com
Wed Nov 13 13:44:05 PST 2013


This is fine for now, but please work on fixing the cross-endian issue so you can remove those XFAILs.

I'm not sure about the exact triples to specify in the XFAIL line.  It might be safer to only specify the base architecture.  For example, we have some tests with "XFAIL: arm" in the test suite now.

Does anyone have an arm system to try reproducing the failure (http://lab.llvm.org:8011/builders/clang-native-arm-cortex-a9/builds/13210)?

On Nov 13, 2013, at 11:29 AM, Yuchen Wu <yuchenericwu at hotmail.com> wrote:

> Once again, I had to revert this patch because the tests were failing on some IBM platforms for reasons I believe are related to endianness. There's also one ARM cortex machine it's failing very peculiarly on, but these are things I'll investigate later.
> 
> For now, here's a patch with the test XFAILing the unsupported platforms.
> 
> ----------------------------------------
>> From: yuchenericwu at hotmail.com
>> To: alp at nuanti.com; mail at justinbogner.com; llvm-commits at cs.uiuc.edu
>> Subject: RE: [PATCH] Added basic testing support for llvm-cov.
>> Date: Mon, 11 Nov 2013 21:04:17 -0500
>> 
>> Hi Alp and Justin,
>> 
>> Thanks for your feedback. I think I will adopt your suggestion of just changing the directory within the test file. I had originally wanted to avoid that because I wasn't sure if there was a precedent for that but given the amount of headaches arising from path parsing, it appears that changing the test file is the simplest approach.
>> 
>> -Yuchen
>> 
>> ----------------------------------------
>>> Date: Tue, 12 Nov 2013 01:14:34 +0000
>>> From: alp at nuanti.com
>>> To: yuchenericwu at hotmail.com; mail at justinbogner.com; llvm-commits at cs.uiuc.edu
>>> Subject: Re: [PATCH] Added basic testing support for llvm-cov.
>>> 
>>> Hello Yuchen,
>>> 
>>> I see what you were trying to do now.
>>> 
>>> You don't need to add extra functionality to llvm-cov for this -- you
>>> can just use 'cd' to change directory directly in the test:
>>> 
>>> |--- a/test/tools/llvm-cov/llvm-cov.test||
>>> ||+++ b/test/tools/llvm-cov/llvm-cov.test||
>>> ||@@ -1,2 +1,2 @@||
>>> ||-RUN: llvm-cov -gcno=test.gcno -gcda=test.gcda -C=%p/Inputs/ \||
>>> ||-RUN: | diff %p/Inputs/test.cpp.gcov -||
>>> ||+RUN: cd %p/Inputs/ && llvm-cov -gcno=test.gcno -gcda=test.gcda \||
>>> ||+RUN: | diff test.cpp.gcov -|
>>> 
>>> 
>>> Your original test passes fine with that change on TOT.
>>> 
>>> Sorry I didn't catch this earlier in review
>>> 
>>> Regards,
>>> Alp.
>>> 
>>> 
>>> On 12/11/2013 00:19, Yuchen Wu wrote:
>>>> ----------------------------------------
>>>>> From: mail at justinbogner.com
>>>>> To: llvm-commits at cs.uiuc.edu
>>>>> CC: llvm-commits at cs.uiuc.edu
>>>>> Subject: Re: [PATCH] Added basic testing support for llvm-cov.
>>>>> Date: Mon, 11 Nov 2013 15:52:18 -0800
>>>>> 
>>>>>> +#include <unistd.h>
>>>>> Pretty sure this is left over from the previous patch - you don't use
>>>>> it.
>>>> Yep, that's my mistake.
>>>> 
>>>>>> using namespace llvm;
>>>>>> 
>>>>>> static cl::opt<bool>
>>>>>> @@ -34,6 +35,9 @@ InputGCDA("gcda", cl::desc("<input gcda file>"), cl::init(""));
>>>>>> static cl::opt<std::string>
>>>>>> OutputFile("o", cl::desc("<output llvm-cov file>"), cl::init("-"));
>>>>>> 
>>>>>> +static cl::opt<std::string>
>>>>>> +WorkingDir("C", cl::desc("change path of working directory"),
>>>>>> + cl::init(""));
>>>>>> 
>>>>>> //===----------------------------------------------------------------------===//
>>>>>> int main(int argc, char **argv) {
>>>>>> @@ -54,8 +58,9 @@ int main(int argc, char **argv) {
>>>>>> errs() << " " << argv[0] << ": No gcov input file!\n";
>>>>>> 
>>>>>> OwningPtr<MemoryBuffer> GCNO_Buff;
>>>>>> - if (error_code ec = MemoryBuffer::getFileOrSTDIN(InputGCNO, GCNO_Buff)) {
>>>>>> - errs() << InputGCNO << ": " << ec.message() << "\n";
>>>>>> + if (error_code ec =
>>>>>> + MemoryBuffer::getFileOrSTDIN(WorkingDir + InputGCNO, GCNO_Buff)) {
>>>>>> + errs() << WorkingDir + InputGCNO << ": " << ec.message() << "\n";
>>>>> I think this only works correctly if the working dir is specified on the
>>>>> command line with a trailing slash. What if this is called with a
>>>>> spelling like "-C /some/dir"?
>>>> I appended an extra "/" to WorkingDir if it's non-empty, should fix that issue.
>>>> 
>>>>>> if (DumpGCOV)
>>>>>> GF.dump();
>>>>>> 
>>>>>> FileInfo FI;
>>>>>> GF.collectLineCounts(FI);
>>>>>> - FI.print(OS, InputGCNO, InputGCDA);
>>>>>> + FI.print(OS, WorkingDir, InputGCNO, InputGCDA);
>>>>> Does this behave strangely if InputGCDA or InputGCNO are absolute paths?
>>>> It depends, without WorkingDir defined, it should be fine. If WorkingDir is defined, then you really shouldn't be using absolute paths for InputGCNO and InputGCDA.
>>>> 
>>>>>> return 0;
>>>>>> }
>>>>> _______________________________________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at cs.uiuc.edu
>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>> 
>>> --
>>> http://www.nuanti.com
>>> the browser experts
>>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits 		 	   		  
> <0012-Added-basic-unit-test-for-llvm-cov.patch>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits




More information about the llvm-commits mailing list