[PATCH v2] tester: Add script to generate html coverage report from covoar output

Cillian O'Donnell cpodonnell8 at gmail.com
Mon Jun 4 20:23:31 UTC 2018 

On Mon, 4 Jun 2018, 21:20 Vijay Kumar Banerjee, <vijaykumar9597 at gmail.com>
wrote:

> On 5 June 2018 at 01:28, Cillian O'Donnell <cpodonnell8 at gmail.com> wrote:
>
>>
>>
>> On 4 June 2018 at 20:29, Vijay Kumar Banerjee <vijaykumar9597 at gmail.com>
>> wrote:
>>
>>> On 5 June 2018 at 00:51, Joel Sherrill <joel at rtems.org> wrote:
>>>
>>>>
>>>>
>>>> On Mon, Jun 4, 2018 at 2:12 PM, Vijay Kumar Banerjee <
>>>> vijaykumar9597 at gmail.com> wrote:
>>>>
>>>>> On 5 June 2018 at 00:31, Joel Sherrill <joel at rtems.org> wrote:
>>>>>
>>>>>> I will add that covoar was originally designed to generate a report
>>>>>> on one
>>>>>> set at a time. The iteration over symbol sets was done at the
>>>>>> scripting
>>>>>> level above that.
>>>>>>
>>>>>> This had the advantage of being simple at the time. It may still be
>>>>>> simple
>>>>>> but moving the iteration over sets to covoar will probably be faster.
>>>>>>
>>>>>> I think DesiredSymbols is instanced a single time. As a starting
>>>>>> point, there
>>>>>> would have to be multiple instances of this -- one for each symbol
>>>>>> set.
>>>>>>
>>>>>> Beyond that, there is a correlation between the report generated and
>>>>>> the desired symbols set.
>>>>>>
>>>>>> So I am thinking that we need to define a "context" structure for each
>>>>>> set. One simple thought is that there is one "master/unified"
>>>>>> DesiredSymbol
>>>>>> set under the hood and the symbols in each set are used as a filter
>>>>>> when generating reports. So process the executables for every symbol
>>>>>> but generate the report subdirectories based on one of the sets of
>>>>>> DesiredSymbols.
>>>>>>
>>>>>> I think that should work.
>>>>>>
>>>>>> Cillian.. you have been through the flow. Am I thinking right that it
>>>>>> is
>>>>>>
>>>>> That sounds like it will work, I'll go back over the covoar code and
>> have a think about it. See if I can find any road blocks.
>>
>>
>>>
>>>>>> And I think we need to merge before doing this type of work. If we
>>>>>> can process
>>>>>> a single set correctly, that's a baseline. Adding iteration will be
>>>>>> easier to review
>>>>>> as another patch.
>>>>>>
>>>>>>
>>>>> we can process a single set correctly.
>>>>> Shall we proceed with merging the above patch with the suggested small
>>>>> edits
>>>>> and then file a ticket for the iteration and then start working on it ?
>>>>>
>>>>
>>>> Please.
>>>>
>>>>>
>>>>> I am confused with running of coverage for multiple bsp. If a single
>>>>> report.html has to show the reports of multiple bsps (as hinted by
>>>>> Cillian)
>>>>>
>>>> Actually I meant separate report.htmls, but if they're all outputed in
>> the the top level directory there'll have to be something internal to know
>> to search for the right output directory with all the coverage data, that's
>> all I meant. Nothing major.
>>
>>
>>> Then a lot of rewroking would be needed. If we're headed in that way
>>>>> then I think making separate report.html like leon3-report.html would
>>>>> be simpler
>>>>> to achieve, and then create a master index.html for listing all the
>>>>> report htmls.
>>>>>
>>>>
>>>> A single run of the tester will test a single BSP. If you open two
>>>> shell windows
>>>> and run the tester in both, will those collide?
>>>>
>>>> In it's current state. Yes, it will collide as all the names of files
>>> (including report.html)
>>> are constant. :(
>>>
>> This is the main fix for now just making adding unique identifiers for
>> files and dirs that will be called in the process. Focus on that, then we
>> can try and merge and the covoar fix for multi sets can come later.
>>
> I have added the update .
> after the update the directory is BSP-coverage ( example :-
> leon3-qemu-coverage)
> they symbol file created is leon3-qemu-symbols.ini
> I have made the necessary changes and leon3-qemu-report.html is showing
> proper results.
>

Perfect! Post patch V3 then and we'll see about this merge.

> covoar does not need support internally to process multiple BSPs
>>>> concurrently.
>>>>
>>>> It is common practice to build and test multiple BSPs in parallel to
>>>> take advantage
>>>> of machines with many cores.
>>>>
>>>> But if you aren't careful, you can't even have two build/test trees at
>>>> the same time
>>>> if they collide on file names in shared directories.
>>>>
>>>> Does that make sense?
>>>>
>>>>
>>>>>> On Mon, Jun 4, 2018 at 1:43 PM, Cillian O'Donnell <
>>>>>> cpodonnell8 at gmail.com> wrote:
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 4 June 2018 at 19:03, Vijay Kumar Banerjee <
>>>>>>> vijaykumar9597 at gmail.com> wrote:
>>>>>>>
>>>>>>>> On 2 June 2018 at 01:00, Vijay Kumar Banerjee <
>>>>>>>> vijaykumar9597 at gmail.com> wrote:
>>>>>>>>
>>>>>>>>> On 2 June 2018 at 00:48, Joel Sherrill <joel at rtems.org> wrote:
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Fri, Jun 1, 2018, 11:21 AM Vijay Kumar Banerjee <
>>>>>>>>>> vijaykumar9597 at gmail.com> wrote:
>>>>>>>>>>
>>>>>>>>>>> On 1 June 2018 at 20:30, Gedare Bloom <gedare at rtems.org> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> On Fri, Jun 1, 2018 at 10:28 AM, Vijay Kumar Banerjee
>>>>>>>>>>>> <vijaykumar9597 at gmail.com> wrote:
>>>>>>>>>>>> > On 1 June 2018 at 19:24, Joel Sherrill <joel at rtems.org>
>>>>>>>>>>>> wrote:
>>>>>>>>>>>> >>
>>>>>>>>>>>> >>
>>>>>>>>>>>> >>
>>>>>>>>>>>> >> On Fri, Jun 1, 2018 at 2:46 AM, Vijay Kumar Banerjee
>>>>>>>>>>>> >> <vijaykumar9597 at gmail.com> wrote:
>>>>>>>>>>>> >>>
>>>>>>>>>>>> >>> Here's the list of Ideas for improvements:
>>>>>>>>>>>> >>>
>>>>>>>>>>>> >>> 1. Include the section coverage in the bsp config file.
>>>>>>>>>>>> >>>     If the section is not found then the script will show
>>>>>>>>>>>> >>>     proper error showing coverage is not supported for the
>>>>>>>>>>>> >>>     provided bsp config file.
>>>>>>>>>>>> >>>
>>>>>>>>>>>> >>> 2. Update covoar to add support for separate coverage report
>>>>>>>>>>>> >>>     for each symbol set.
>>>>>>>>>>>> >>>
>>>>>>>>>>>> >>> 3. Add a method somewhere in covoar to get the size of an
>>>>>>>>>>>> instruction
>>>>>>>>>>>> >>>     and fix the hard coded size 4 in ObjdumpProcessor.cc
>>>>>>>>>>>> >>
>>>>>>>>>>>> >>
>>>>>>>>>>>> >> What about a single XXX_run command? What about that
>>>>>>>>>>>> suggestion?
>>>>>>>>>>>> >>
>>>>>>>>>>>> > The suggestion was to turn test_run and coverage_run into a
>>>>>>>>>>>> single command.
>>>>>>>>>>>> > I have kept them separate so that there's a possibility to
>>>>>>>>>>>> just run the
>>>>>>>>>>>> > test.
>>>>>>>>>>>> >
>>>>>>>>>>>> > If we want to run coverage everytime we run the test. we can
>>>>>>>>>>>> do it.
>>>>>>>>>>>> > Then I think the --coverage option can be changed to
>>>>>>>>>>>> --coverage-sets
>>>>>>>>>>>> > to mention the sets.
>>>>>>>>>>>> > If that's what we're looking for then I don't think a
>>>>>>>>>>>> separate ticket is
>>>>>>>>>>>> > needed,
>>>>>>>>>>>> > I can try to do it within tomorrow and submit an updated
>>>>>>>>>>>> patch.
>>>>>>>>>>>> >
>>>>>>>>>>>> >>
>>>>>>>>>>>> >> Will there be an update to this patch?
>>>>>>>>>>>> >>
>>>>>>>>>>>> > This patch is working an showing results. I don't have any
>>>>>>>>>>>> work
>>>>>>>>>>>> > going related to this patch currently.
>>>>>>>>>>>> > If there are any suggestions, I'll try to include all the
>>>>>>>>>>>> suggested updates
>>>>>>>>>>>> > as soon as possible and submit. So that we can get it merged.
>>>>>>>>>>>> >
>>>>>>>>>>>>
>>>>>>>>>>>> I get confused by the similarity between test_run() and
>>>>>>>>>>>> coverage_run()
>>>>>>>>>>>> names, and now I'm also seeing some confusion because there is a
>>>>>>>>>>>> function coverage_run() and a class coverage_run. I suggest you
>>>>>>>>>>>> remove
>>>>>>>>>>>> this function coverage_run(), and make either
>>>>>>>>>>>> coverage_run.__init__()
>>>>>>>>>>>> or coverage_run.run() take the executables as a parameter
>>>>>>>>>>>> directly.
>>>>>>>>>>>>
>>>>>>>>>>>> Thank you for the suggestion. :)
>>>>>>>>>>> I have removed the function and taken executables as a parameter
>>>>>>>>>>> in
>>>>>>>>>>> coverage_run.__init__()
>>>>>>>>>>>
>>>>>>>>>>> I have a question.
>>>>>>>>>>> The ini file that is fed to covoar is written by the script
>>>>>>>>>>> according to the
>>>>>>>>>>> symbols mentioned by the user. I haven't include the ini file in
>>>>>>>>>>> the patch.
>>>>>>>>>>> I'm planning to delete the file after the run, unless --no-clean
>>>>>>>>>>> option is given,
>>>>>>>>>>> which currently deletes the .cov trace files after the run.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> That makes sense.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> Can I proceed with this ?
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Yes.
>>>>>>>>>>
>>>>>>>>> Added. Thanks. :)
>>>>>>>>>
>>>>>>>>>> also, shall I include that in the .gitignore ?
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> 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.
>>>>>>>>>>
>>>>>>>>>> The ini file I'm talking about is the symbol sets config file not
>>>>>>>>> the bsp
>>>>>>>>> config file. yes the name is constant. Should it be unique to the
>>>>>>>>> bsp ?
>>>>>>>>> something like, leon3-symbols.ini ?
>>>>>>>>> How does the automated testing work?
>>>>>>>>>
>>>>>>>>>> I think the name needs to be unique enough.to 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.
>>>>>>>>>>
>>>>>>>>>>> If it is needed then I have a fix in mind. I can take the bsp
>>>>>>>>> name and add
>>>>>>>>> '-symbols.ini' to it. and add *-symbols.ini to .gitignore .
>>>>>>>>>
>>>>>>>> Shall I add this or put a fixme in the code and post a patch ?
>>>>>>>> Are there any other suggestions for the patch ?
>>>>>>>>
>>>>>>>> I was looking into covoar for generating separate reports for each
>>>>>>>> symbolset.
>>>>>>>> Seems like  all the coverage reports are generated together and are
>>>>>>>> written
>>>>>>>> in the _outputDirectory_ .
>>>>>>>>
>>>>>>>
>>>>>>> The output directory should be aligned with the bsp and the
>>>>>>> report.html changed to keep record of this instead of searching for the
>>>>>>> generic coverage dir. Look for leon3-coverage and so on. As well as the
>>>>>>> symbol-sets.ini change you mentioned above. That would probably be enough
>>>>>>> to not cause any conflicts with parallel testing (I may be missing a case
>>>>>>> there, I let you know if anything else comes to mind). The main thing to
>>>>>>> think about is if multiple bsps are being tested at the same time, they
>>>>>>> have to know which config file is there's and which output directory and
>>>>>>> whatever else they may be looking for, the names have to be unique. These
>>>>>>> things are all fed to covoar so the changes can be added to coverage.py.
>>>>>>>
>>>>>>> I couldn't figure out how to cleanly address this.
>>>>>>>> If covoar is intended to generate reports from multiple
>>>>>>>> subsystems/symbolsets,
>>>>>>>> then I think this would be a needed update. Otherwise we can do it
>>>>>>>> from the
>>>>>>>> script, by feeding covoar with a single set ini and putting the
>>>>>>>> result in a separate
>>>>>>>> directory .
>>>>>>>>
>>>>>>>>> Can we do this ?
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>> -Gedare
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> devel mailing list
>>>>>>>> devel at rtems.org
>>>>>>>> http://lists.rtems.org/mailman/listinfo/devel
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20180604/cfea328d/attachment-0001.html>

More information about the devel mailing list