Vivek <vivek@collab.net> writes:
> [[[
> Create a new file for testing date related issues.
>
> *subversion/tests/clients/cmdline/date_tests.py: New File.
> ]]]
Don't forget the space after the "*".
By the way, if you're including the log message in the body of the
mail message, then don't include a separate patch containing a log
message. And if you *do* put the log message in an attachment, just
put it in the same attachment as the code patch itself, using the "[[[
... ]]]" as delimiters. That way the log message can't get separated
from its patch.
Isn't this a revised version of an earlier patch? If so, put "v2" or
"v3" or whatever's appropriate in the Subject line.
> Index: subversion/tests/clients/cmdline/date_tests.py
> ===================================================================
> --- subversion/tests/clients/cmdline/date_tests.py (revision 0)
> +++ subversion/tests/clients/cmdline/date_tests.py (revision 0)
> @@ -0,0 +1,176 @@
> +#!/usr/bin/env python
> +#
> +# date_tests.py: testing date cases.
> +#
> +# Subversion is a tool for revision control.
> +# See http://subversion.tigris.org for more information.
> +#
> +# ====================================================================
> +# Copyright (c) 2000-2004 CollabNet. All rights reserved.
> +#
> +# This software is licensed as described in the file COPYING, which
> +# you should have received as part of this distribution. The terms
> +# are also available at http://subversion.tigris.org/license-1.html.
> +# If newer versions of this license are posted there, you may use a
> +# newer version instead, at your option.
> +#
> +######################################################################
Copyright should just say "2005", not "2000-2004".
> +# General modules
> +import os, time
> +
> +# Our testing module
> +import svntest
> +
> +# (abbreviation)
> +Skip = svntest.testcase.Skip
> +XFail = svntest.testcase.XFail
> +Item = svntest.wc.StateItem
> +
> +#----------------------------------------------------------------------
> +# Is Subversion A Day Early? Yes, so modify test time accordingly.
> +
> +t = list(time.localtime())
> +t[2] += 2 #day
> +t[3] += 2 #hour
> +t = tuple(t)
Heh, some people might disagree with that comment, but whatever :-)
If 't' were only a helper variable used in creating 'time_array'
below, I wouldn't object to having such a cryptic name for a top-level
variable. But it is also used in some of the test functions. In that
case, please give it a more comprehensible name, like 'local_time' or
something.
> +time_array = [
> + time.strftime('%Y-%m-%d', t),
> + time.strftime('%H:%M', t),
> + time.strftime('%Y-%m-%d %H:%M', t),
> + time.strftime("%H:%M:%S", t)+'.000000',
> + time.strftime('%Y%m%dT%H%M', t),
> + time.strftime('%Y%m%dT%H%M%z', t),
> + time.strftime('%Y-%m-%dT%H:%M%z', t),
> + time.strftime('%Y-%m-%d %H:%M %z', t),
> + time.strftime('%Y-%m-%dT%H:%M', t),
> + time.strftime('%Y-%m-%d %H:%M', t),
> + ]
> +
> +######################################################################
> +# Tests
> +#
> +# Each test must return on success or raise on failure.
> +
> +#----------------------------------------------------------------------
> +
> +#----------------------------------------------------------------------
> +# Test log with date option.
> +def log_with_date_option(sbox):
> + "get log with -r {date}"
> + sbox.build()
> +
> + wc_dir = sbox.wc_dir
> +
> + mu_path = os.path.join(wc_dir, 'A', 'mu')
> +
> + for i in range(0, len(time_array)):
> + out, err = svntest.main.run_svn(1, 'log', mu_path, '-r',
> + '{' + str(time_array[i]) + '}'
> + )
> + if not(err == [] and out[3] == 'Log message for revision 1.\n'):
> + raise svntest.Failure
This is the only test that uses time_array, therefore shouldn't
time_array be a local variable within the test?
On Windows, 'svn log' ends lines with CRLF instead of just LF. Does
your comparison above still work then (and if so, why? :-) ) ?
> +#----------------------------------------------------------------------
> +# The output of GNU date was not being accepted by svn commands
> +# Test the fix.
> +def cat_with_date_option(sbox):
> + "use GNU date command in revision input"
> + sbox.build()
> +
> + wc_dir = sbox.wc_dir
> +
> + mu_path = os.path.join(wc_dir, 'A', 'mu')
> +
> + # The Success Case: output and no errors.
> + arg = os.popen("date --iso-8601=seconds -d '+2 hours'").readline()
> + svntest.actions.run_and_verify_svn('cat a file using GNU date',
> + ["This is the file 'mu'."], None,
> + 'cat', '-r', '{' +
> + str(arg).strip('\n') + '}', mu_path)
> +
> + # The Failure Case: no output and some errors.
> + arg = os.popen("date --iso-8601=seconds -d '-2 hours'").readline()
> + svntest.actions.run_and_verify_svn('cat a file using GNU date',
> + [], svntest.SVNAnyOutput, 'cat', '-r',
> + '{' + str(arg).strip('\n') + '}',
> + mu_path)
I seem to remember some previous discussion on the list about whether
it was better to invoke 'date' externally, or better to construct a
GNU-date-formatted date manually. Ah, I found the thread here
http://subversion.tigris.org/servlets/BrowseList?list=dev&by=thread&from=336009
and I see that strftime() is not portable anyway. Too bad.
But, still, there are going to be some systems on which 'date' is
either not installed, or does not support the flags you're passing to
it. The test should not fail on those systems, it should just "SKIP".
In the 'test_list' later on, you address this by testing whether or
not os.name == 'win32'. Is that really the best way? (I'm asking,
not asserting that it's not.)
Why not see whether popen() failed, within the test function itself?
If it fails, the test certainly shouldn't fail. It could at least
succeed, or even better, you could see if
raise svntest.Skip
does the right thing (but I haven't tested that).
By the way, note that r15191 changed the default content of the test
files. A/mu would now have "This is the file 'mu'.\n" (the newline
wasn't there before r15191).
> +#----------------------------------------------------------------------
> +# Date is stored as an unversiond property in the reop. If we try
> +# to meddle with it, we may land in trouble. Here is a test.
> +def cat_with_date_prop_modified(sbox):
> + "change the date property"
Fitz already corrected the "reop" ==> "repos" typo in
http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=101523
:-)
> + sbox.build()
> + mu_path = os.path.join(sbox.wc_dir, 'A', 'mu')
> +
> + # Create the revprop-change hook
> + if os.name == 'posix':
> + hook = os.path.join(svntest.main.current_repo_dir, 'hooks',
> + 'pre-revprop-change')
> + svntest.main.file_append(hook, "#!/bin/sh\n\nexit 0\n")
> + os.chmod(hook, 0755)
> + elif sys.platform == 'win32':
> + hook = os.path.join(svntest.main.current_repo_dir,
> + 'hooks', 'pre-revprop-change.bat')
> + svntest.main.file_append(hook, "@exit 0\n")
> +
This is interesting, I don't think we've had any automated hook
testing yet, but this strategy seems to work. Does it pass for you on
both Unix and Win32 systems? I'd be okay with it; it would be nice to
abstract it out so other systems can test hooks too. David Anderson
was talking about hook testing for his svnserve path-authz changes
earlier today.
Again, as Fitz also asked, why the svntest.main.file_append() when
open().write() would do? I know you swiped it from another test; but
IMHO just because one test does it doesn't mean we have to perpetuate
it :-). This is about writing, not appending, so let's just do the
most straightforward thing.
> + # Test: everything ok?
> + arg = time.strftime('%Y-%m-%d %H:%M', t)
> + svntest.actions.run_and_verify_svn('Cat a file with svn:date property set',
> + ["This is the file 'mu'."], [],
> + 'cat', '-r',
> + '{' + str(arg) + '}', mu_path)
I believe you can pass 'None' instead of '[]', and if so, that would
be a better flag value.
> + # Set: property svn:date with a bogus date
> + out = ["property 'svn:date' set on repository revision 1\n"]
> + svntest.actions.run_and_verify_svn(None, out, [], 'propset',
> + "--revprop", "-r", '1',
> + '-R', 'svn:date',
> + time.strftime('%Y-%m-%dT%H:%M:%S'),
> + sbox.wc_dir)
Same about [] vs None.
> + e_out, err = svntest.main.run_svn (1, 'cat', '-r',
> + '{' + str(arg) + '}', mu_path)
> + if err.pop() != 'svn: Bogus date\n':
> + raise svntest.Failure
What does "e_out" signify, how is it different from "out"?
I recommend against the .pop() idiom. If someone went to examine err
later, it wouldn't have the same data. Instead, just index into err[].
> + # We did a mistake in setting up the date.
> + # Can we reset it? Format is in libsvn_subr/time.c
> + svntest.actions.run_and_verify_svn(None, out, [],
> + 'propset', "--revprop", "-r", '1',
> + '-R', 'svn:date',
> + time.strftime('%Y-%m-%dT%H:%M:%S')
> + + '.000000Z',
> + sbox.wc_dir)
> +
> + # Yes, we can!
> + svntest.actions.run_and_verify_svn('Cat a file with svn:date property set',
> + ["This is the file 'mu'."], [],
> + 'cat', '-r',
> + '{' + str(arg) + '}', mu_path)
> +
Nice test, I like the strategy!
> +########################################################################
> +# Run the tests
> +
> +
> +# list all tests here, starting with None:
> +
> +test_list = [ None,
> + log_with_date_option,
> + Skip(cat_with_date_option, os.name == 'win32'),
> + # If we learn how to write a pre-revprop-change hook for
> + # non-Posix platforms, we won't have to skip here:
> + Skip(cat_with_date_prop_modified,
> + (os.name != 'posix' and sys.platform != 'win32')),
> + ]
That comment seems a little out of date. I mean, you've got the hook
testing working for win32 as well as Posix...
> +if __name__ == '__main__':
> + svntest.main.run_tests(test_list)
> + # NOTREACHED
> +
> +
> +### End of file.
Okay.
The log message attachment said:
> Create a new file for testing date related issues.
>
> * subversion/tests/clients/cmdline/date_tests.py: New File.
See, here the log message differs from the one in the body of your
message -- here it has the space after the asterisk. If there were
only one copy, I'd know which one to review :-).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Jul 1 22:37:20 2005