RFR: JDK-8199681 Remove boilerplate code from creating native jtreg tests (original) (raw)

David Holmes david.holmes at oracle.com
Fri Mar 16 13:06:43 UTC 2018

On 16/03/2018 9:43 PM, Magnus Ihse Bursie wrote:

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 ($(OPENJDKTARGETOS), windows) SRC += mytest endif Now you have to write ifneq ($(OPENJDKTARGETOS), windows) EXCLUDE += mytest endif Seriously, that's not much harder.

True. It just doesn't look that clean in the actual file.

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.

True.

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.

Given we used to include directories it seems natural to me that we now exclude directories. Though I suppose files does give more fine grained control if we were to need it.

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

Let's just see how it goes, as you've already pushed it.

Thanks, David

49 BUILDHOTSPOTJTREGIMAGEDIR := $(TESTIMAGEDIR)/hotspot/jtreg I thought you'd changed something here then noticed that we end up doing:  _94     DEST := $(TESTIMAGEDIR)/hotspot/jtreg/native, _ and BUILDHOTSPOTJTREGIMAGEDIR 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 COMPAREBUILD. 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

More information about the build-dev mailing list