[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR 9: 8035889: jdk testlibrary - add printing of values of failed assertions
From:       Staffan Larsen <staffan.larsen () oracle ! com>
Date:       2014-02-28 9:43:20
Message-ID: 6A397A15-56DD-41FA-885F-4A3BF2415CB6 () oracle ! com
[Download RAW message or body]

Looks good to me!

Thanks,
/Staffan

On 27 feb 2014, at 16:34, roger riggs <roger.riggs@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 
> > > > > 
> > > > > 
> > > > 
> > > 
> > 
> 


[Attachment #3 (unknown)]

<html><head><meta http-equiv="Content-Type" content="text/html \
charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: \
space; -webkit-line-break: after-white-space;"><div>Looks good to \
me!</div><div><br></div><div>Thanks,</div><div>/Staffan</div><div><br></div><div><div>On \
27 feb 2014, at 16:34, roger riggs &lt;<a \
href="mailto:roger.riggs@oracle.com">roger.riggs@oracle.com</a>&gt; wrote:</div><br \
class="Apple-interchange-newline"><blockquote type="cite">  
    <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
  
  <div bgcolor="#FFFFFF" text="#000000">
    Hi Mandy,<br>
    <br>
    I updated the webrev:&nbsp; <br>
    &nbsp; <a moz-do-not-send="true" class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/%7Erriggs/webrev-testlibrary-asserts-8035889/">http://cr.openjdk.java.net/~rriggs/webrev-testlibrary-asserts-8035889/</a>
  <br>
    <br>
    Alan suggested copying serviceability-dev so they have a chance to
    review if desired.<br>
    <br>
    I want to investigate if it is possible to use the TestNG Assert
    classes without<br>
    the TestNG execution framework.&nbsp; <br>
    It would be necessary to compile/run against TestNG.jar but it might
    not <br>
    need the entire mechanism.<br>
    <br>
    Thanks, Roger<br>
    <br>
    <div class="moz-cite-prefix">On 2/26/2014 10:17 PM, Mandy Chung
      wrote:<br>
    </div>
    <blockquote cite="mid:530EAE5F.3050308@oracle.com" type="cite">
      <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
      On 2/26/2014 7:09 PM, Roger Riggs wrote:<br>
      <blockquote cite="mid:530EAC78.4000301@Oracle.com" type="cite">
        <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
        Hi Mandy,<br>
        <br>
        Yes, it might be more productive to switch the tests to TestNG.<br>
        But it did provide support in cases where TestNG could not be
        used, <br>
        for example in a directory of existing tests that had custom
        reporting.<br>
        <br>
        But I remember there is a problem with TestNG having a
        dependency for XML<br>
        which is not supported in Profile1 and a number of tests had to
        be disabled<br>
        in that configuration.&nbsp; Will XML always be available.&nbsp; Do we
        need to solve<br>
        or work around that problem with TestNG?<br>
        <br>
      </blockquote>
      <br>
      This is a good point.&nbsp;&nbsp; When we want to test just the base module
      for example, how can we run TestNG tests?&nbsp; We need to address that
      certainly.<br>
      <br>
      My comment on TestNG is a question for new tests using this
      Asserts class.&nbsp; Your patch is fine to go (after taking out
      @library tag if I got it correct). <br>
      <br>
      Mandy<br>
      <br>
      <blockquote cite="mid:530EAC78.4000301@Oracle.com" type="cite">
        Thanks, Roger<br>
        <br>
        <div class="moz-cite-prefix">On 2/26/14 9:01 PM, Mandy Chung
          wrote:<br>
        </div>
        <blockquote cite="mid:530E9C6C.8080409@oracle.com" type="cite">Hi

          Roger, <br>
          <br>
          On 2/26/2014 12:34 PM, roger riggs wrote: <br>
          <blockquote type="cite">The testlibrary for the jdk should be
            printing the values in the failed <br>
            assertions to make debugging easier and quicker. <br>
            <br>
            The webrev adds the printing of the failed assertions and
            added methods <br>
            for formatting and unconditional fail methods. <br>
            <br>
            Webrev: <br>
            <a moz-do-not-send="true" class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/%7Erriggs/webrev-testlibrary-asserts-8035889/">http://cr.openjdk.java.net/~rriggs/webrev-testlibrary-asserts-8035889/</a>
  <br>
            <br>
          </blockquote>
          <br>
          AssertsTest.java: line 28:&nbsp; @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. <br>
          <br>
          Otherwise, the change looks okay. <br>
          <br>
          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).&nbsp; Are you writing new tests using this
          Asserts class? <br>
          <br>
          Mandy <br>
          <br>
          <blockquote type="cite">Bug: <br>
            &nbsp;&nbsp; 8035889: jdk testlibrary - add printing of values of
            failed assertions <br>
            <br>
            Thanks, Roger <br>
            <br>
            [1] <a moz-do-not-send="true" class="moz-txt-link-freetext" \
href="https://bugs.openjdk.java.net/browse/JDK-8035889">https://bugs.openjdk.java.net/browse/JDK-8035889</a>
  <br>
            <br>
            <br>
          </blockquote>
          <br>
        </blockquote>
        <br>
      </blockquote>
      <br>
    </blockquote>
    <br>
  </div>

</blockquote></div><br></body></html>



[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic