On Mon, 4 Jun 2018, 21:20 Vijay Kumar Banerjee, <vijaykumar9...@gmail.com> wrote:
> On 5 June 2018 at 01:28, Cillian O'Donnell <cpodonne...@gmail.com> wrote: > >> >> >> On 4 June 2018 at 20:29, Vijay Kumar Banerjee <vijaykumar9...@gmail.com> >> wrote: >> >>> On 5 June 2018 at 00:51, Joel Sherrill <j...@rtems.org> wrote: >>> >>>> >>>> >>>> On Mon, Jun 4, 2018 at 2:12 PM, Vijay Kumar Banerjee < >>>> vijaykumar9...@gmail.com> wrote: >>>> >>>>> On 5 June 2018 at 00:31, Joel Sherrill <j...@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 < >>>>>> cpodonne...@gmail.com> wrote: >>>>>> >>>>>>> >>>>>>> >>>>>>> On 4 June 2018 at 19:03, Vijay Kumar Banerjee < >>>>>>> vijaykumar9...@gmail.com> wrote: >>>>>>> >>>>>>>> On 2 June 2018 at 01:00, Vijay Kumar Banerjee < >>>>>>>> vijaykumar9...@gmail.com> wrote: >>>>>>>> >>>>>>>>> On 2 June 2018 at 00:48, Joel Sherrill <j...@rtems.org> wrote: >>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On Fri, Jun 1, 2018, 11:21 AM Vijay Kumar Banerjee < >>>>>>>>>> vijaykumar9...@gmail.com> wrote: >>>>>>>>>> >>>>>>>>>>> On 1 June 2018 at 20:30, Gedare Bloom <ged...@rtems.org> wrote: >>>>>>>>>>> >>>>>>>>>>>> On Fri, Jun 1, 2018 at 10:28 AM, Vijay Kumar Banerjee >>>>>>>>>>>> <vijaykumar9...@gmail.com> wrote: >>>>>>>>>>>> > On 1 June 2018 at 19:24, Joel Sherrill <j...@rtems.org> >>>>>>>>>>>> wrote: >>>>>>>>>>>> >> >>>>>>>>>>>> >> >>>>>>>>>>>> >> >>>>>>>>>>>> >> On Fri, Jun 1, 2018 at 2:46 AM, Vijay Kumar Banerjee >>>>>>>>>>>> >> <vijaykumar9...@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@rtems.org >>>>>>>> http://lists.rtems.org/mailman/listinfo/devel >>>>>>>> >>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> >
_______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel