RFR : 8006594/8006595 : Define jdk_core test set
mike.duigou at oracle.com
Mon Feb 4 16:02:34 UTC 2013
On Feb 4 2013, at 05:54 , Alan Bateman wrote:
> On 02/02/2013 20:40, Mike Duigou wrote:
>> I have updated both the jdk repo patch:
>> 8006594: Add jdk_core target to jdk/test/Makefile
>> and the root repo patch:
>> 8006595: Use jdk/test/Makefile targets in preference to local definitions
>> The primary difference between this version and the previous is the judicious use of "-k" option and silencing of command spew. A top-level make test TEST=jdk_core now succeeds even if sub-tests fail.
>> I investigated changing the behaviour which determines the test output location so that all jdk_core output would be in a single directory. I have opted to not change it yet as I can't be certain what depends upon the current behaviour.
>> I would like to change one additional aspect--currently the "prep" target forces the output dir to be wiped via a dependency on "clean". I would like to remove this dependency and not wipe the output dir with each run. For RE and JPRT uses they are likely to be running in fresh directories anyway so this change should only impact developers who likely wouldn't want the complete wipe anyway.
>> There are some things about the jtreg_tests target which seem very specific to how JPRT uses this makefile. Not all of the behaviours, such as bundling of test output into zip files, make sense for all likely users of this makefile. I will work with Kelly to partition the JPRT functionality and better understand how this Makefile is used by JPRT for future patches.
> If I understand correctly, then the default will now be jdk_core + langtools but no hotspot (just checking that this is what is intended).
Correct. This is actually unchanged. I am not sure if we should redefine the default to do a broad but shallow smoke test of all repos. I believe this would be the most useful default. The jdk_core set can be expanded/altered if we would like a wider cross section of tests run.
> I agree that having JPRT config in the repo is confusing, especially with the *.test.targets properties that overlap/conflict with the make files.
For now I don't want to break/change any existing usages.
> common/makefiles/Main.gmk - it looks like you've added CONCURRENCY=$(JOBS). Is $(JOBS) coming from the --num-cores specified to configure?
> If so then I think we have to careful because -concurrency means a lot of virtual memory and I'm not convinced that we limit it via -vmoption in jdk/test/Makefile
> I could see us wanting to dial this down on 32-bit Windows for example.
We do limit the max size via -Xmx512M which does seem quite small and is generally well balanced with the number of cores.
I opened a build/infrastructure issue to have MEMORY_SIZE and perhaps some suggested VM sizes recorded to the spec.gmk I planned to revisit this again once only new build is supported.
> Also I'm not 100% sure that jdk/test/TEST.ROOT is up to date for the client area -- that's the place where excludeAccess.dirs lists the directories with tests that cannot run concurrently.
The makefile currently segregates tests to use othervm or agentvm by target ie. jdk_beans1 . I haven't checked whether these are synchronized against TEST.ROOT and indeed it would be a shame if these had not been kept in sync. (This is why I added notes about "keep this in sync with ..." in various places) I will check this with Stuart Marks today and plan remediation if necessary.
> The webrev has an updated to webrev.ksh, I assume we should ignore that.
Correct, spillover from 8004726 patch. (which also needs review. :-) )
> In jdk/test/Makefile you have silenced the jtreg command but I think that is very useful to have in the log files.
It does produce a lot of spew but I will unsilence it. When the old build is retired we can integrate the new build logging control and make this controllable.
> Minor nit in jdk/test/Makefile but the jdk_core recipe could be split over multiple lines to avoid the very long line.
I will do so.
> Otherwise I think this looks okay to me. As I mentioned in the original mail I do think we need to move to the point where the mappings is not in the make files and hopefully a future installation will get us moving in that direction.
More information about the core-libs-dev