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. > 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