<div dir="auto"><div><br><br><div class="gmail_quote"><div dir="ltr">On Fri, Jun 1, 2018, 11:21 AM Vijay Kumar Banerjee <<a href="mailto:vijaykumar9597@gmail.com">vijaykumar9597@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On 1 June 2018 at 20:30, Gedare Bloom <span dir="ltr"><<a href="mailto:gedare@rtems.org" target="_blank" rel="noreferrer">gedare@rtems.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Fri, Jun 1, 2018 at 10:28 AM, Vijay Kumar Banerjee<br>
<div><div class="m_-5676231831902086972h5"><<a href="mailto:vijaykumar9597@gmail.com" target="_blank" rel="noreferrer">vijaykumar9597@gmail.com</a>> wrote:<br>
> On 1 June 2018 at 19:24, Joel Sherrill <<a href="mailto:joel@rtems.org" target="_blank" rel="noreferrer">joel@rtems.org</a>> wrote:<br>
>><br>
>><br>
>><br>
>> On Fri, Jun 1, 2018 at 2:46 AM, Vijay Kumar Banerjee<br>
>> <<a href="mailto:vijaykumar9597@gmail.com" target="_blank" rel="noreferrer">vijaykumar9597@gmail.com</a>> wrote:<br>
>>><br>
>>> Here's the list of Ideas for improvements:<br>
>>><br>
>>> 1. Include the section coverage in the bsp config file.<br>
>>>     If the section is not found then the script will show<br>
>>>     proper error showing coverage is not supported for the<br>
>>>     provided bsp config file.<br>
>>><br>
>>> 2. Update covoar to add support for separate coverage report<br>
>>>     for each symbol set.<br>
>>><br>
>>> 3. Add a method somewhere in covoar to get the size of an instruction<br>
>>>     and fix the hard coded size 4 in ObjdumpProcessor.cc<br>
>><br>
>><br>
>> What about a single XXX_run command? What about that suggestion?<br>
>><br>
> The suggestion was to turn test_run and coverage_run into a single command.<br>
> I have kept them separate so that there's a possibility to just run the<br>
> test.<br>
><br>
> If we want to run coverage everytime we run the test. we can do it.<br>
> Then I think the --coverage option can be changed to --coverage-sets<br>
> to mention the sets.<br>
> If that's what we're looking for then I don't think a separate ticket is<br>
> needed,<br>
> I can try to do it within tomorrow and submit an updated patch.<br>
><br>
>><br>
>> Will there be an update to this patch?<br>
>><br>
> This patch is working an showing results. I don't have any work<br>
> going related to this patch currently.<br>
> If there are any suggestions, I'll try to include all the suggested updates<br>
> as soon as possible and submit. So that we can get it merged.<br>
><br>
<br>
</div></div>I get confused by the similarity between test_run() and coverage_run()<br>
names, and now I'm also seeing some confusion because there is a<br>
function coverage_run() and a class coverage_run. I suggest you remove<br>
this function coverage_run(), and make either coverage_run.__init__()<br>
or coverage_run.run() take the executables as a parameter directly.<br>
<span class="m_-5676231831902086972HOEnZb"><font color="#888888"><br></font></span></blockquote><div>Thank you for the suggestion. :)</div><div>I have removed the function and taken executables as a parameter in</div><div>coverage_run.__init__() </div><div><br></div><div>I have a question.</div><div>The ini file that is fed to covoar is written by the script according to the</div><div>symbols mentioned by the user. I haven't include the ini file in the patch.</div><div>I'm planning to delete the file after the run, unless --no-clean option is given,</div><div>which currently deletes the .cov trace files after the run.</div></div></div></div></blockquote></div></div><div dir="auto"><br></div><div dir="auto">That makes sense.</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><div>Can I proceed with this ? </div></div></div></div></blockquote></div></div><div dir="auto"><br></div><div dir="auto">Yes.</div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>also, shall I include that in the .gitignore ?</div></div></div></div></blockquote></div></div><div dir="auto"><br></div><div dir="auto">Is the name of the file constant? The same across multiple BSPs? If so, then this will be a problem doing automated testing of multiple BSPs in parallel.</div><div dir="auto"><br></div><div dir="auto">I think the name needs to be unique <a href="http://enough.to">enough.to</a> support running testing with coverage on multiple BSPs in parallel. That means you can't add it to gitignore. And can add another issue and FIXME to the code.</div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="m_-5676231831902086972HOEnZb"><font color="#888888">
-Gedare<br>
</font></span></blockquote></div><br></div></div>
</blockquote></div></div></div>