Hi Torsten, We experienced some similar problems with our builds at Mercap, because all the tests involving code analysis are orders of magnitude slower than the unit tests. What we do is: - When the programmer closes a unit of work (in that case it will be equivalent to create a PR) we run the tests involving this kind of code analysis only over the changed code - In the global build this tests are run in parallel (this wouldn't solve the pressure over the infrastructure, but improves the troughput of the whole build)
I think this kind of tests totally worth the effort. Gabriel On Thu, Sep 6, 2018 at 12:51 PM Torsten Bergmann <[email protected]> wrote: > Hi, > > the very nice contribution on GIFReadWriter/animated images (see > https://pharo.manuscript.com/f/cases/22137) > unfortunately introduced some methods that had no sender, were > uncategorized and had unused temporary variables, etc. > > Something that is meanwhile shown by our tools and easy to fix. I guess it > was some overseen leftover. Unfortunately this > also slipped through the integration process as it was merged without an > approval from a community reviewer. > > I dont want to lament too much on this specifc case (especially as I now > fixed it in https://pharo.fogbugz.com/f/cases/22426). > > But after I cleaned the P7 image this spring to not have unused > temporaries anymore I was suprised to find such > glitches again now. So to me it shows that we need to write more tests to > detect violations and keep our achieved quality standards > up and also should not solely rely on the review process. > > Otherwise we need to invest our time in cleaning up our own stuff > afterwards again and again. > > To be specific one can evaluate: > > CompiledMethod allInstances select: [:m | m ast temporaries > anySatisfy: [:x | x binding isUnused ]]. > > to find unused temps - which would be easy to package up into one of the > release test to ensure no unused temp vars are left > (the tearDown would require an "ASTCache reset"). > > The problem is that this simple test would take around 20-30 seconds > depending on machine and I do not know if it would be a > good idea/good solution to integrate. Especially I'm not sure if it is > worth the issue or will kill the CI again and again. > > What do you think about it? Comments appreciated? > > Thanks > T. > > > > >
