RFR: JDK-8199681 Remove boilerplate code from creating native jtreg tests

[Magnus Ihse Bursie]

On 2018-03-16 03:48, David Holmes wrote:
> 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, :) )

Previously you had to write like:
ifeq ($(OPENJDK_TARGET_OS), windows)
SRC += mytest
endif

Now you have to write
ifneq ($(OPENJDK_TARGET_OS), windows)
EXCLUDE += mytest
endif

Seriously, that's not much harder.

But the latter has the extremely important benefit that for all 
multi-platform tests (which comprise the absolute majority), you don't 
have to modify the makefiles at all.

> Also having to exclude on the file level is more awkward than using 
> the directory level.
I can of course change that. I chose the file level due to these reasons:
* We have a requirement of filename uniqueness across all tests (since 
they compile to object files in a single directory); however we do not 
have a requirement of directory name uniqueness.
* Basing this on filenames allows you to single out individual tests 
that are, for logical reasons, grouped together in a single directory.
* And it was easier to implement in make scripts. :-)

As a suggestion, I can add a PATTERN_EXCLUDE. This way it will work 
about the same as the Setup*Compilation functions, were you can exclude 
based on file name or pattern matching. Do you want me to do that? I 
think with the current number of excludes, it's not really worth it, but 
if it is important to you, I can fix it.

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

*duh*

I have already pushed the patch. I will fix this in a follow-up bug. Let 
me know if you want to have a directory-based or pattern- based 
exclusion mechanism in that as well.

/Magnus

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