Hi David, > So all such tests should be using driver mode, and further the VMs they then > exec don't use any of the APIs that include the jtreg test arguments.
correct, and 8151707's subtasks are going to mark only such tests (and tests which should be using driver-mode, but can't due to external factors, remember these follow-up fixes for my use driver-mode? ;) ). there are two more (a bit controversial) use cases where we can consider usage of vm.flagless: - some of debugger-debuggee tests have debugger executed w/ external flags, but don't pass these flags to debuggee; and in most cases, it doesn't seem to be right, so arguable all such tests should be updated to use driver mode to run debugger and then marked w/ vm.flagless. I know that svc team was doing some cleanup in this area recently, and given it's require more investigation w.r.t the tests' intent, I don't plan to do it as a part of 8151707, and instead will create follow up RFEs/tasks. - a unit-like tests which don't ignore flags, but weren't designed to be run w/ external flags; most of jfr tests can be used as an example: you can run w/ any flags, but they might fail as they assert things which happen only in certain configurations and these configurations are written in jtreg test descriptions. currently, these tests are marked w/ jfr k/w and it's advised not to run them w/ any external flags, yet I know that some people successfully do that to test their configurations. given the set of configurations which satisfies needs of jfr tests is much bigger than the configurations listed in the tests, I kinda feel sympathetic to people doing that, on the other hand, it's unsupported and I'd prefer us to express (and enforce) that more clearly. again, given the possible controversiality and need for a broader discussion, I'm planning to file an issue for jfr tests and follow up later w/ interested parties. to sum up, 8151707's subtasks are going to mark *only* obvious and non-controversial cases. for all other cases, the JBS entries are to be filed and followed up on. Cheers, -- Igor > On Jun 3, 2020, at 4:02 PM, David Holmes <david.hol...@oracle.com> wrote: > > Hi Igor, > > On 4/06/2020 7:30 am, Igor Ignatyev wrote: >> http://cr.openjdk.java.net/~iignatyev//8246494/webrev.00 >>> 70 lines changed: 66 ins; 0 del; 4 mod >> Hi all, >> could you please review the patch which introduces a new @requires property >> to filter out the tests which ignore externally provided JVM flags? >> the idea behind this patch is to have a way to clearly mark tests which >> ignore flags, so >> a) it's obvious that they don't execute a flag-guarded code/feature, and >> extra care should be taken to use them to verify any flag-guarded changed; >> b) they can be easily excluded from runs w/ flags. > > So all such tests should be using driver mode, and further the VMs they then > exec don't use any of the APIs that include the jtreg test arguments. > > Okay this seems reasonable in what it does. > > Thanks, > David > >> @requires and VMProps allows us to achieve both, so it's been decided to add >> a new property `vm.flagless`. `vm.flagless` is set to false if there are any >> XX flags other than `-XX:MaxRAMPercentage` and `-XX:CreateCoredumpOnCrash` >> (which are known to be set almost always) or any X flags other `-Xmixed`; in >> other words any tests w/ `@requires vm.flagless` will be excluded from runs >> w/ any other X / XX flags passed via `-vmoption` / `-javaoption`. in rare >> cases, when one still wants to run the tests marked by `vm.flagless` w/ >> external flags, `vm.flagless` can be forcefully set to true by setting any >> value to `TEST_VM_FLAGLESS` env. variable. >> this patch adds necessary common changes and marks common tests, namely >> Scimark, GTestWrapper and TestNativeProcessBuilder. Component-specific tests >> will be marked separately by the corresponding subtasks of 8151707[1]. >> please note, the patch depends on CODETOOLS-7902336[2], which will be >> included in the next jtreg version, so this patch is to be integrated only >> after jtreg5.1 is promoted and we switch to use it by 8246387[3]. >> JBS: https://bugs.openjdk.java.net/browse/JDK-8246494 >> webrev: http://cr.openjdk.java.net/~iignatyev//8246494/webrev.00 >> testing: marked tests w/ different XX and X flags w/ and w/o >> TEST_VM_FLAGLESS env. var, and w/o any flags >> [1] https://bugs.openjdk.java.net/browse/JDK-8151707 >> [2] https://bugs.openjdk.java.net/browse/CODETOOLS-7902336 >> [3] https://bugs.openjdk.java.net/browse/JDK-8246387 >> Thanks, >> -- Igor
