Looks good to me! Thanks, /Staffan
On 27 feb 2014, at 16:34, roger riggs <roger.ri...@oracle.com> wrote: > Hi Mandy, > > I updated the webrev: > http://cr.openjdk.java.net/~rriggs/webrev-testlibrary-asserts-8035889/ > > Alan suggested copying serviceability-dev so they have a chance to review if > desired. > > I want to investigate if it is possible to use the TestNG Assert classes > without > the TestNG execution framework. > It would be necessary to compile/run against TestNG.jar but it might not > need the entire mechanism. > > Thanks, Roger > > On 2/26/2014 10:17 PM, Mandy Chung wrote: >> On 2/26/2014 7:09 PM, Roger Riggs wrote: >>> Hi Mandy, >>> >>> Yes, it might be more productive to switch the tests to TestNG. >>> But it did provide support in cases where TestNG could not be used, >>> for example in a directory of existing tests that had custom reporting. >>> >>> But I remember there is a problem with TestNG having a dependency for XML >>> which is not supported in Profile1 and a number of tests had to be disabled >>> in that configuration. Will XML always be available. Do we need to solve >>> or work around that problem with TestNG? >>> >> >> This is a good point. When we want to test just the base module for >> example, how can we run TestNG tests? We need to address that certainly. >> >> My comment on TestNG is a question for new tests using this Asserts class. >> Your patch is fine to go (after taking out @library tag if I got it >> correct). >> >> Mandy >> >>> Thanks, Roger >>> >>> On 2/26/14 9:01 PM, Mandy Chung wrote: >>>> Hi Roger, >>>> >>>> On 2/26/2014 12:34 PM, roger riggs wrote: >>>>> The testlibrary for the jdk should be printing the values in the failed >>>>> assertions to make debugging easier and quicker. >>>>> >>>>> The webrev adds the printing of the failed assertions and added methods >>>>> for formatting and unconditional fail methods. >>>>> >>>>> Webrev: >>>>> http://cr.openjdk.java.net/~rriggs/webrev-testlibrary-asserts-8035889/ >>>>> >>>> >>>> AssertsTest.java: line 28: @library doesn't look like it's needed. There >>>> is no jdk/test/testlibrary directory and I think >>>> jdk.testlibrary.* are found as relative to $test.src. >>>> >>>> Otherwise, the change looks okay. >>>> >>>> Now that jtreg supports TestNG and I wonder if this class should retire >>>> some day (there are only about 10 existing tests using this class). Are >>>> you writing new tests using this Asserts class? >>>> >>>> Mandy >>>> >>>>> Bug: >>>>> 8035889: jdk testlibrary - add printing of values of failed assertions >>>>> >>>>> Thanks, Roger >>>>> >>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8035889 >>>>> >>>>> >>>> >>> >> >
