Re: RFR(S): 8162727 - Testbug: additional requires properties can't be used for filtering server vm in jdk jtreg tests
> On Aug 4, 2016, at 3:56 PM, Dmitry Fazunenko > wrote: > > Igor, > > Regardless this change I agree with you: VM specific tests should be moved > from jdk to hotspot. > > But we can't get rid of VM specific tests in the jdk repository at all. Tests > for java.lang.management belong to JDK, but may use VM specific... > I count 11 open and 100 closed jdk tests which use @requires vm.*. > Well, I looked thru them and almost all of them look as hotspot tests. > I don't see s a big problem in having a few lines duplicated in TEST.ROOT. We > don't duplicate the code, but repeat a sort of "import" statements. > So, I'm okay with the proposed change. the problem is not in code duplication, it is in making that easy to have hotspot tests in jdk. > > Thanks, > Dima > > > On 04.08.2016 11:46, Igor Ignatyev wrote: >> Dima, >> >> I’d prefer to have tests moved into hotspot repo instead. none of jdk tests >> should depend on vm kind or any other details of vm. your change just makes >> things worse, we will not just have tests which depend on vm implementation >> details in jdk, we will also have an mechanism for other tests to express >> such dependency, which can lead to increased number of “hotspot” tests in >> jdk. we went similar path w/ testlibrary and we are still paying for that. >> >> so how many tests which need these properties do we have now? >> >> Thanks, >> — Igor >> >>> On Aug 3, 2016, at 4:25 PM, Dmitrij Pochepko >>> wrote: >>> >>> Hi, >>> >>> can somebody with official reviewer role review this? >>> >>> Thanks, >>> Dmitrij >>> >>> On 29.07.2016 21:42, Dmitrij Pochepko wrote: Sure, now including core-libs-dev Thanks, Dmitrij On 29.07.2016 20:16, joe darcy wrote: > Hello, > > I also recommend sending this review request over to core-libs-dev since > it affect the jdk repo. > > Thanks, > > -Joe > > > On 7/29/2016 12:35 AM, Dmitry Fazunenko wrote: >> Hi Dmitrij, >> >> The change itself looks good. >> >> One note: this change adds a little overhead (a very little) for running >> tests in jdk repository, but it's required only for VM specific tests >> stored in jdk. As alternative of this change I can suggest moving VM >> specific tests from the 'jdk' to 'hotspot' repository. Perhaps, such >> massive update is too late for JDK9 time frame and could be done only in >> JDK10. >> So, if the changes depending on 8162727 are not so critical and could be >> deferred I would prefer to postpone this fix. >> >> Thanks, >> Dima >> >> >> On 29.07.2016 4:29, Vladimir Kozlov wrote: >>> It affects all groups. Should be reviewed on hotspot-dev. >>> >>> Thanks, >>> Vladimir >>> >>> On 7/28/16 10:19 AM, Dmitrij Pochepko wrote: Hi, please review small patch for 8162727 - Testbug: additional requires properties can't be used for filtering server vm in jdk jtreg tests A problem is that tests under jdk repo can't use additional jtreg requires properties(like vm.flavor to filter out client or server vm). The same functionality was introduced for hotspot repo as part of JDK-8154956. So, in order to filter tests using these additional properties a small fix is created. webrev: http://cr.openjdk.java.net/~dpochepk/8162727/webrev.01/ CR: https://bugs.openjdk.java.net/browse/JDK-8162727 Thanks, Dmitrij >
Re: RFR(S): 8162727 - Testbug: additional requires properties can't be used for filtering server vm in jdk jtreg tests
Igor, Regardless this change I agree with you: VM specific tests should be moved from jdk to hotspot. But we can't get rid of VM specific tests in the jdk repository at all. Tests for java.lang.management belong to JDK, but may use VM specific... I count 11 open and 100 closed jdk tests which use @requires vm.*. I don't see s a big problem in having a few lines duplicated in TEST.ROOT. We don't duplicate the code, but repeat a sort of "import" statements. So, I'm okay with the proposed change. Thanks, Dima On 04.08.2016 11:46, Igor Ignatyev wrote: Dima, I’d prefer to have tests moved into hotspot repo instead. none of jdk tests should depend on vm kind or any other details of vm. your change just makes things worse, we will not just have tests which depend on vm implementation details in jdk, we will also have an mechanism for other tests to express such dependency, which can lead to increased number of “hotspot” tests in jdk. we went similar path w/ testlibrary and we are still paying for that. so how many tests which need these properties do we have now? Thanks, — Igor On Aug 3, 2016, at 4:25 PM, Dmitrij Pochepko wrote: Hi, can somebody with official reviewer role review this? Thanks, Dmitrij On 29.07.2016 21:42, Dmitrij Pochepko wrote: Sure, now including core-libs-dev Thanks, Dmitrij On 29.07.2016 20:16, joe darcy wrote: Hello, I also recommend sending this review request over to core-libs-dev since it affect the jdk repo. Thanks, -Joe On 7/29/2016 12:35 AM, Dmitry Fazunenko wrote: Hi Dmitrij, The change itself looks good. One note: this change adds a little overhead (a very little) for running tests in jdk repository, but it's required only for VM specific tests stored in jdk. As alternative of this change I can suggest moving VM specific tests from the 'jdk' to 'hotspot' repository. Perhaps, such massive update is too late for JDK9 time frame and could be done only in JDK10. So, if the changes depending on 8162727 are not so critical and could be deferred I would prefer to postpone this fix. Thanks, Dima On 29.07.2016 4:29, Vladimir Kozlov wrote: It affects all groups. Should be reviewed on hotspot-dev. Thanks, Vladimir On 7/28/16 10:19 AM, Dmitrij Pochepko wrote: Hi, please review small patch for 8162727 - Testbug: additional requires properties can't be used for filtering server vm in jdk jtreg tests A problem is that tests under jdk repo can't use additional jtreg requires properties(like vm.flavor to filter out client or server vm). The same functionality was introduced for hotspot repo as part of JDK-8154956. So, in order to filter tests using these additional properties a small fix is created. webrev: http://cr.openjdk.java.net/~dpochepk/8162727/webrev.01/ CR: https://bugs.openjdk.java.net/browse/JDK-8162727 Thanks, Dmitrij
Re: RFR(S): 8162727 - Testbug: additional requires properties can't be used for filtering server vm in jdk jtreg tests
Hi, I'm aware of one test only so far. Shoud I move this single test or a whole testgroup which this test belongs to? Thanks, Dmitrij Dima, I’d prefer to have tests moved into hotspot repo instead. none of jdk tests should depend on vm kind or any other details of vm. your change just makes things worse, we will not just have tests which depend on vm implementation details in jdk, we will also have an mechanism for other tests to express such dependency, which can lead to increased number of “hotspot” tests in jdk. we went similar path w/ testlibrary and we are still paying for that. so how many tests which need these properties do we have now? Thanks, — Igor On Aug 3, 2016, at 4:25 PM, Dmitrij Pochepko wrote: Hi, can somebody with official reviewer role review this? Thanks, Dmitrij On 29.07.2016 21:42, Dmitrij Pochepko wrote: Sure, now including core-libs-dev Thanks, Dmitrij On 29.07.2016 20:16, joe darcy wrote: Hello, I also recommend sending this review request over to core-libs-dev since it affect the jdk repo. Thanks, -Joe On 7/29/2016 12:35 AM, Dmitry Fazunenko wrote: Hi Dmitrij, The change itself looks good. One note: this change adds a little overhead (a very little) for running tests in jdk repository, but it's required only for VM specific tests stored in jdk. As alternative of this change I can suggest moving VM specific tests from the 'jdk' to 'hotspot' repository. Perhaps, such massive update is too late for JDK9 time frame and could be done only in JDK10. So, if the changes depending on 8162727 are not so critical and could be deferred I would prefer to postpone this fix. Thanks, Dima On 29.07.2016 4:29, Vladimir Kozlov wrote: It affects all groups. Should be reviewed on hotspot-dev. Thanks, Vladimir On 7/28/16 10:19 AM, Dmitrij Pochepko wrote: Hi, please review small patch for 8162727 - Testbug: additional requires properties can't be used for filtering server vm in jdk jtreg tests A problem is that tests under jdk repo can't use additional jtreg requires properties(like vm.flavor to filter out client or server vm). The same functionality was introduced for hotspot repo as part of JDK-8154956. So, in order to filter tests using these additional properties a small fix is created. webrev: http://cr.openjdk.java.net/~dpochepk/8162727/webrev.01/ CR: https://bugs.openjdk.java.net/browse/JDK-8162727 Thanks, Dmitrij
Re: RFR(S): 8162727 - Testbug: additional requires properties can't be used for filtering server vm in jdk jtreg tests
Dima, I’d prefer to have tests moved into hotspot repo instead. none of jdk tests should depend on vm kind or any other details of vm. your change just makes things worse, we will not just have tests which depend on vm implementation details in jdk, we will also have an mechanism for other tests to express such dependency, which can lead to increased number of “hotspot” tests in jdk. we went similar path w/ testlibrary and we are still paying for that. so how many tests which need these properties do we have now? Thanks, — Igor > On Aug 3, 2016, at 4:25 PM, Dmitrij Pochepko > wrote: > > Hi, > > can somebody with official reviewer role review this? > > Thanks, > Dmitrij > > On 29.07.2016 21:42, Dmitrij Pochepko wrote: >> Sure, >> >> now including core-libs-dev >> >> Thanks, >> Dmitrij >> >> On 29.07.2016 20:16, joe darcy wrote: >>> Hello, >>> >>> I also recommend sending this review request over to core-libs-dev since it >>> affect the jdk repo. >>> >>> Thanks, >>> >>> -Joe >>> >>> >>> On 7/29/2016 12:35 AM, Dmitry Fazunenko wrote: Hi Dmitrij, The change itself looks good. One note: this change adds a little overhead (a very little) for running tests in jdk repository, but it's required only for VM specific tests stored in jdk. As alternative of this change I can suggest moving VM specific tests from the 'jdk' to 'hotspot' repository. Perhaps, such massive update is too late for JDK9 time frame and could be done only in JDK10. So, if the changes depending on 8162727 are not so critical and could be deferred I would prefer to postpone this fix. Thanks, Dima On 29.07.2016 4:29, Vladimir Kozlov wrote: > It affects all groups. Should be reviewed on hotspot-dev. > > Thanks, > Vladimir > > On 7/28/16 10:19 AM, Dmitrij Pochepko wrote: >> Hi, >> >> please review small patch for 8162727 - Testbug: additional requires >> properties can't be used for filtering server vm in jdk jtreg tests >> >> A problem is that tests under jdk repo can't use additional jtreg >> requires properties(like vm.flavor to filter out client or server vm). >> The same functionality was introduced for hotspot repo as part >> of JDK-8154956. So, in order to filter tests using these additional >> properties a small fix is created. >> >> webrev: http://cr.openjdk.java.net/~dpochepk/8162727/webrev.01/ >> CR: https://bugs.openjdk.java.net/browse/JDK-8162727 >> >> Thanks, >> Dmitrij >>> >> >
Re: RFR(S): 8162727 - Testbug: additional requires properties can't be used for filtering server vm in jdk jtreg tests
Hi David, On 03.08.2016 23:56, David Holmes wrote: Hi Dmitrij, I know this reflects the changes made in hotspot/test but I have a query. Does this: + requires.extraPropDefns.vmOpts = -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI -Xbootclasspath/a:bootClasses mean that the VM under test is executed with those additional flags when running the tests? Or is it run once with those flags to establish the "requires" properties? That flags are used only once for establishing the "requires" properties. Thanks, Dima If the latter then I have no concern and this change seems fine. Thanks, David On 3/08/2016 11:25 PM, Dmitrij Pochepko wrote: Hi, can somebody with official reviewer role review this? Thanks, Dmitrij On 29.07.2016 21:42, Dmitrij Pochepko wrote: Sure, now including core-libs-dev Thanks, Dmitrij On 29.07.2016 20:16, joe darcy wrote: Hello, I also recommend sending this review request over to core-libs-dev since it affect the jdk repo. Thanks, -Joe On 7/29/2016 12:35 AM, Dmitry Fazunenko wrote: Hi Dmitrij, The change itself looks good. One note: this change adds a little overhead (a very little) for running tests in jdk repository, but it's required only for VM specific tests stored in jdk. As alternative of this change I can suggest moving VM specific tests from the 'jdk' to 'hotspot' repository. Perhaps, such massive update is too late for JDK9 time frame and could be done only in JDK10. So, if the changes depending on 8162727 are not so critical and could be deferred I would prefer to postpone this fix. Thanks, Dima On 29.07.2016 4:29, Vladimir Kozlov wrote: It affects all groups. Should be reviewed on hotspot-dev. Thanks, Vladimir On 7/28/16 10:19 AM, Dmitrij Pochepko wrote: Hi, please review small patch for 8162727 - Testbug: additional requires properties can't be used for filtering server vm in jdk jtreg tests A problem is that tests under jdk repo can't use additional jtreg requires properties(like vm.flavor to filter out client or server vm). The same functionality was introduced for hotspot repo as part of JDK-8154956. So, in order to filter tests using these additional properties a small fix is created. webrev: http://cr.openjdk.java.net/~dpochepk/8162727/webrev.01/ CR: https://bugs.openjdk.java.net/browse/JDK-8162727 Thanks, Dmitrij
Re: RFR(S): 8162727 - Testbug: additional requires properties can't be used for filtering server vm in jdk jtreg tests
Hi Dmitrij, I know this reflects the changes made in hotspot/test but I have a query. Does this: + requires.extraPropDefns.vmOpts = -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI -Xbootclasspath/a:bootClasses mean that the VM under test is executed with those additional flags when running the tests? Or is it run once with those flags to establish the "requires" properties? If the latter then I have no concern and this change seems fine. Thanks, David On 3/08/2016 11:25 PM, Dmitrij Pochepko wrote: Hi, can somebody with official reviewer role review this? Thanks, Dmitrij On 29.07.2016 21:42, Dmitrij Pochepko wrote: Sure, now including core-libs-dev Thanks, Dmitrij On 29.07.2016 20:16, joe darcy wrote: Hello, I also recommend sending this review request over to core-libs-dev since it affect the jdk repo. Thanks, -Joe On 7/29/2016 12:35 AM, Dmitry Fazunenko wrote: Hi Dmitrij, The change itself looks good. One note: this change adds a little overhead (a very little) for running tests in jdk repository, but it's required only for VM specific tests stored in jdk. As alternative of this change I can suggest moving VM specific tests from the 'jdk' to 'hotspot' repository. Perhaps, such massive update is too late for JDK9 time frame and could be done only in JDK10. So, if the changes depending on 8162727 are not so critical and could be deferred I would prefer to postpone this fix. Thanks, Dima On 29.07.2016 4:29, Vladimir Kozlov wrote: It affects all groups. Should be reviewed on hotspot-dev. Thanks, Vladimir On 7/28/16 10:19 AM, Dmitrij Pochepko wrote: Hi, please review small patch for 8162727 - Testbug: additional requires properties can't be used for filtering server vm in jdk jtreg tests A problem is that tests under jdk repo can't use additional jtreg requires properties(like vm.flavor to filter out client or server vm). The same functionality was introduced for hotspot repo as part of JDK-8154956. So, in order to filter tests using these additional properties a small fix is created. webrev: http://cr.openjdk.java.net/~dpochepk/8162727/webrev.01/ CR: https://bugs.openjdk.java.net/browse/JDK-8162727 Thanks, Dmitrij
Re: RFR(S): 8162727 - Testbug: additional requires properties can't be used for filtering server vm in jdk jtreg tests
Hi, can somebody with official reviewer role review this? Thanks, Dmitrij On 29.07.2016 21:42, Dmitrij Pochepko wrote: Sure, now including core-libs-dev Thanks, Dmitrij On 29.07.2016 20:16, joe darcy wrote: Hello, I also recommend sending this review request over to core-libs-dev since it affect the jdk repo. Thanks, -Joe On 7/29/2016 12:35 AM, Dmitry Fazunenko wrote: Hi Dmitrij, The change itself looks good. One note: this change adds a little overhead (a very little) for running tests in jdk repository, but it's required only for VM specific tests stored in jdk. As alternative of this change I can suggest moving VM specific tests from the 'jdk' to 'hotspot' repository. Perhaps, such massive update is too late for JDK9 time frame and could be done only in JDK10. So, if the changes depending on 8162727 are not so critical and could be deferred I would prefer to postpone this fix. Thanks, Dima On 29.07.2016 4:29, Vladimir Kozlov wrote: It affects all groups. Should be reviewed on hotspot-dev. Thanks, Vladimir On 7/28/16 10:19 AM, Dmitrij Pochepko wrote: Hi, please review small patch for 8162727 - Testbug: additional requires properties can't be used for filtering server vm in jdk jtreg tests A problem is that tests under jdk repo can't use additional jtreg requires properties(like vm.flavor to filter out client or server vm). The same functionality was introduced for hotspot repo as part of JDK-8154956. So, in order to filter tests using these additional properties a small fix is created. webrev: http://cr.openjdk.java.net/~dpochepk/8162727/webrev.01/ CR: https://bugs.openjdk.java.net/browse/JDK-8162727 Thanks, Dmitrij
Re: RFR(S): 8162727 - Testbug: additional requires properties can't be used for filtering server vm in jdk jtreg tests
Sure, now including core-libs-dev Thanks, Dmitrij On 29.07.2016 20:16, joe darcy wrote: Hello, I also recommend sending this review request over to core-libs-dev since it affect the jdk repo. Thanks, -Joe On 7/29/2016 12:35 AM, Dmitry Fazunenko wrote: Hi Dmitrij, The change itself looks good. One note: this change adds a little overhead (a very little) for running tests in jdk repository, but it's required only for VM specific tests stored in jdk. As alternative of this change I can suggest moving VM specific tests from the 'jdk' to 'hotspot' repository. Perhaps, such massive update is too late for JDK9 time frame and could be done only in JDK10. So, if the changes depending on 8162727 are not so critical and could be deferred I would prefer to postpone this fix. Thanks, Dima On 29.07.2016 4:29, Vladimir Kozlov wrote: It affects all groups. Should be reviewed on hotspot-dev. Thanks, Vladimir On 7/28/16 10:19 AM, Dmitrij Pochepko wrote: Hi, please review small patch for 8162727 - Testbug: additional requires properties can't be used for filtering server vm in jdk jtreg tests A problem is that tests under jdk repo can't use additional jtreg requires properties(like vm.flavor to filter out client or server vm). The same functionality was introduced for hotspot repo as part of JDK-8154956. So, in order to filter tests using these additional properties a small fix is created. webrev: http://cr.openjdk.java.net/~dpochepk/8162727/webrev.01/ CR: https://bugs.openjdk.java.net/browse/JDK-8162727 Thanks, Dmitrij
