RFR(M): 8189762: [TESTBUG] Create tests for JDK-8146115 container awareness and resource configuration
Mikhailo Seledtsov
mikhailo.seledtsov at oracle.com
Thu Nov 9 01:26:41 UTC 2017
Here is an update webrev:
http://cr.openjdk.java.net/~mseledtsov/8189762.02/
I also tried to generate a diff between 01 and 02, but could not. My
script does not seem to work any more, or I am missing something.
Igor, please let me know if you have scripts or command line to generate
incremental webrev (I committed my initial changes locally, then applied
the new changes on top).
Summary for updated webrev.02:
- implemented review feedback from Bob and Igor
- added @driver to all tests, since the actual testing happens in the
child docker process; the main is just a test driver
- configured docker directory for exclusive access by jtreg tests,
since there are potential and actual issues when running these tests
concurrently
- modified test groups to add a hotspot_docker test group, and to
exclude docker testing from lower tiers
(once the infra is ready, I plan to add this execution initially at
higher tiers; first will run it for a while as custom runs)
My next steps:
- rebase to new jdk tree
- update/merge
- retest
Thank you,
Misha
On 11/6/17, 9:20 PM, Mikhailo Seledtsov wrote:
> Hi Igor,
>
> Thank you for reviewing the code.
>
> On 11/6/17, 4:31 PM, Igor Ignatyev wrote:
>> Hi Misha,
>>
>> I have several comments:
>> 1. whitebox.cpp : WB_IsContainerized has an incorrect signature, it
>> should be WB_IsContainerized(JNIEnv*, jobject)
> Fixed
>> 2. CPUSetsReader.java : listToString can be rewritten using stream
>> api as
>> list.stream().limit(maxCount).map(Object::toString).collect(Collectors.joining(","))
> Thank you. I will try it out.
>> 3. in several files, I have noticed some problems w/ indents which
>> make code quite hard to read, for example
>>> 73 Common.run( Common.newOpts(imageName)
>>> 74 .addDockerOpts("--memory", valueToSet))
>>> 75 .shouldMatch("Memory Limit is:.*" +
>>> expectedTraceValue);
>> or
>>> 96 DockerRunOptions opts = Common.newOpts(imageName,
>>> "AttemptOOM")
>>> 97 .addDockerOpts("--memory", dockerMemLimit,
>>> "--memory-swap", dockerMemLimit);
>>> 98 opts.classParams.add("" + sizeToAllocInMb);
> OK. I will unwind these expressions, and make sure indentation looks
> good.
>> 4. TestCPUSets.java : it's better to use
>> Assert.assertEquals(parts.length, 2) than
>> Asserts.assertTrue(parts.length == 2) as the former will also print
>> the actual value of parts.length. so you won't need to have L#103
> OK
>> 5. Test*: there is no need to have parentheses in '@requires
>> (docker.support)'
> OK
>> 6. Test*: ClassFileInstaller doesn't have to be run by JDK under
>> test, so you can use '@run driver' to run it
> Will fix.
>> 7. TestCPUSets.java : L#72, to get a proper new line symbol you
>> should use %n in format string. also System.out::printf seems to be
>> more popular in our code than System.out::format
> OK.
>> 8. TestCPUSets.java: shouldn't checkResult assert that we found
>> lineMarker?
> Oops. Missed it. Will add code to check it.
>> 9. Test*: would you consider adding .shouldHaveExitValue(0) to all
>> Common.run calls which aren't expected to fail, e.g. all test* in
>> TestCPUAwareness, testMemorySoftLimit and testMemoryLimit in
>> TestMemoryAwareness
> Common.run() already checks for .shouldHaveExitValue(0) after running
> the container.
>
>
> I will apply feedback from Bob and you, and will post a new webrev.
>
>
> Thank you,
> Misha
>>
>> Thanks,
>> -- Igor
>>
>>> On Nov 1, 2017, at 8:11 PM, mikhailo<mikhailo.seledtsov at oracle.com>
>>> wrote:
>>>
>>> Please review these tests that were developed to test JVM's
>>> container awareness in Docker environment.
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8189762
>>> Webrev:
>>> Tests: http://cr.openjdk.java.net/~mseledtsov/8189762.00/
>>> WB API:
>>> http://cr.openjdk.java.net/~mseledtsov/8189762.00.whitebox/
>>> Testing:
>>> 1. Locally: Linux-x64, docker engine version: 17.06.2-ce
>>> Ran the developed tests via jtreg
>>> Pass
>>>
>>> 2. Automated testing system - run these tests
>>> In progress
>>>
>>> Thank you,
>>> Misha
>>>
More information about the hotspot-runtime-dev
mailing list