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

Vijay Kumar Banerjee vijaykumar9597 at gmail.com
Mon Jun 4 20:20:07 UTC 2018 

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.

> 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/20180605/6f009e94/attachment-0002.html>

More information about the devel mailing list