RFR: 8159370: Add FlagGuard for easier modification of flags for unit tests

Kim Barrett kim.barrett at oracle.com
Tue Jun 21 16:54:31 UTC 2016


> On Jun 21, 2016, at 10:27 AM, Erik Helin <erik.helin at oracle.com> wrote:
> 
> On 2016-06-17, Kim Barrett wrote:
> Thanks for correcting the URL. New patches are located at:
> - incremental: http://cr.openjdk.java.net/~ehelin/8159370/01-02/
> - full: http://cr.openjdk.java.net/~ehelin/8159370/02/

Looks good.

>> test/native/runtime/test_globals.cpp 
>> 
>> This file only contains tests for FlagGuard.  I don't know what
>> conventions we're adopting for native tests, and what restrictions the
>> build system might be imposing on us.  It seems to me though that this
>> might be better called test_FlagGuard, and other native tests of
>> facilities in globals.hpp would be in other file(s).
> 
> I would like to start by using one test_ file per source file until we
> have more experience with the unit tests. If we then find that it is
> better to use multiple test_ files for one source files, then I'll be
> happy to move the FlagGuard tests into its own file.

OK.

>> test/native/runtime/test_globals.cpp 
>> 
>> The code here is extreme boilerplate.  Consider a macro to reduce
>> that?
> 
> Yep, agree. What do you think of the macro?

I think I would have included the TEST_VM(…) in the macro expansion,
but I’m OK with it as is too, since I can come up with arguments for
preferring the separation.  In particular, if a test fails, making the
failing test easier to find (without needing to understand a macro) by
reported name has some benefit.



More information about the hotspot-dev mailing list