Hi Magnus,
Removing boilerplate is good but the exclude mechanism seems rather
awkward compared to the previous include. It's much more obvious when
writing a platform specific native test to include it for that platform,
rather than exclude it for all other platforms. You run the risk of
having excludes spread all over the place, or not knowing where you
should put them. I thought the opt-in approach of just adding the test
directory to the list of native test directories was simple and worked
well. (The need to add the extra link instructions wasn't good though, :) )
Also having to exclude on the file level is more awkward than using the
directory level.
49 BUILD_HOTSPOT_JTREG_IMAGE_DIR := $(TEST_IMAGE_DIR)/hotspot/jtreg
I thought you'd changed something here then noticed that we end up doing:
94 DEST := $(TEST_IMAGE_DIR)/hotspot/jtreg/native, \
and BUILD_HOTSPOT_JTREG_IMAGE_DIR is actually unused.
Thanks,
David
On 15/03/2018 11:01 PM, Magnus Ihse Bursie wrote:
There's currently a lot of boilerplate code needed to setup a new native
jtreg test. This was never the way it was intended to work -- the idea
was that you basically should just add the file and then things should
work automatically, unless you had special requirements.
This patch will make it so once more.
I have tested this using COMPARE_BUILD. I get some spurious binary
differences on macosx. I can't really say why, but then again, we have
never verified the test image by a clean "back-to-back" null comparison
either, so I'm not worried. Apart from that, it looks green.
Bug: https://bugs.openjdk.java.net/browse/JDK-8199681
WebRev:
http://cr.openjdk.java.net/~ihse/JDK-8199681-remote-native-test-boilerplate/webrev.01
/Magnus
