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 >>> >>> 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) >> 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. :(
> 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