<html>
  <head>
    <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    Thomas,<br>
    <br>
      This is the new version:<br>
    <a href="http://cr.openjdk.java.net/%7Eminqi/7164841/webrev05"> 
      http://cr.openjdk.java.net/~minqi/7164841/webrev05</a><br>
      <br>
      The change is adding internal tests after make_log_name_internal
    added to implement the original make_log_name function.<br>
    <br>
      Please let me know if it is OK. JPRT task is under
    building/testing, so far it is good.<br>
    <br>
    Thanks<br>
    Yumin<br>
    <br>
    <div class="moz-cite-prefix">On 9/12/2013 8:24 AM, Thomas Schatzl
      wrote:<br>
    </div>
    <blockquote cite="mid:1378999468.2692.120.camel@cirrus" type="cite">
      <pre wrap="">Hi Yumin,

On Wed, 2013-09-11 at 15:45 -0700, Yumin Qi wrote:
</pre>
      <blockquote type="cite">
        <pre wrap="">Thomas,

   Attached are two files, on is script file for running and the other 
is the output result.
   With this flag

ExecuteInternalVMTests

on, the run only print out so much, and did not contine to do rest of 
the test. Think it is right.
</pre>
      </blockquote>
      <pre wrap="">
  sorry for being unclear: the request has been to add new tests to the
internal VM tests, not only run them. The method to replace the %p/%t
markers would make them an ideal candidate for such testing.

E.g. have a method that compares output of that function with some known
values.

However I did not think it through completely, as the values of %p/%t
are dependent on external circumstances, and it is maybe not worth to
add infrastructure to allow fake values (well, one could extract the
generation of these values out, and pass them to the function under
test).

I.e.  change make_log_name()

to this signature:

make_log_name_internal(const char* log_name, const char*
force_directory, const char* datetime, int pid) {

  ... use datetime, pid in the code instead of retrieving them here...
}

make_log_name(const char* log_name, const char* force_directory) {
  char tms[32];
  get_datetime_string(tms, sizeof(tms);

  make_log_name_internal(log_name, force_directory, tms,
os::current_process_id());
}

then you could easily write tests like:

  char* fixed_time = "1980-20-....";
  char* result;

  result = make_log_name_internal("somename%p", false, fixed_time, 200);
  if (strncmp(result, "somename200") != 0) { // error out }
  free result

  [... more tests...]

and so on. I am not sure how you did the testing while developing, but
the effort would probably only be a matter of converting them to this
format.

Thomas


</pre>
    </blockquote>
    <br>
  </body>
</html>