You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <d....@daniel.shahaf.name> on 2010/07/24 10:30:10 UTC

Re: svn commit: r978841 - in /subversion/trunk/subversion/tests/cmdline: svnrdump_tests.py svnrdump_tests_data/ svnrdump_tests_data/revision0.dump

artagnon@apache.org wrote on Sat, Jul 24, 2010 at 10:18:58 -0000:
> +  # Create a dump file using svnrdump
> +  r, svnrdump_dumpfile, err = svntest.main.run_svnrdump(sbox.repo_url)
> +
> +  # Check error code
> +  if (r != 0):
> +    raise svntest.Failure('Result code not 0')
> +
> +  # Check the output from stderr
> +  if not err[0].startswith('* Dumped revision'):
> +    raise svntest.Failure('No valid output')

Not your fault, but that's not what I meant.  What I meant was to check for no
unexpected stderr (e.g., no "svn: warning %s" or similar).

For example, you could do that by running 'svnrdump -q' and then verifying that
*nothing* was printed to stderr.

Makes sense?

Re: svn commit: r978841 - in /subversion/trunk/subversion/tests/cmdline: svnrdump_tests.py svnrdump_tests_data/ svnrdump_tests_data/revision0.dump

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Ramkumar Ramachandra wrote on Sat, Jul 24, 2010 at 16:58:34 +0530:
> Hi Daniel,
> 
> Daniel Shahaf writes:
> > Please don't add "Review by" before I've actually reviewed the patch, thanks :-)
> 
> Um, right. I guess I misunderstood you when you asked me to put the
> 'Review by' in your previous email. I'll include it while actually
> committing, but not in the email.
> 

It's not a "in email" versus "in the commit", but rather "before I reviewed"
and "after I reviewed".

I ask you not to write "Review by: danielsh" until *after* I have reviewed the
diff (not just the idea; the actual diff).  Whether it's in email or in the
physical log message is immaterial

Clearer?

> > Perhaps you can use one of the UnexpectedStderr classes?  This has the
> > advantage that the unexpected stderr output would be printed to whomever is
> > running the test.
> 
> Excellent suggestion! Now fixed.
> 
> [[[
> * subversino/tests/cmdline/svnrdump_tests.py (run_test): Run svnrdump
>   with '-q' and check that nothing is printed to stderr.
> ]]]
> 
> Index: subversion/tests/cmdline/svnrdump_tests.py
> ===================================================================
> --- subversion/tests/cmdline/svnrdump_tests.py	(revision 978841)
> +++ subversion/tests/cmdline/svnrdump_tests.py	(working copy)
> @@ -73,15 +73,15 @@ def run_test(sbox, dumpfile_name):
>    svntest.actions.run_and_verify_load(sbox.repo_dir, svnadmin_dumpfile)
>  
>    # Create a dump file using svnrdump
> -  r, svnrdump_dumpfile, err = svntest.main.run_svnrdump(sbox.repo_url)
> +  r, svnrdump_dumpfile, err = svntest.main.run_svnrdump('-q', sbox.repo_url)
>  
>    # Check error code
>    if (r != 0):
>      raise svntest.Failure('Result code not 0')
>  
>    # Check the output from stderr
> -  if not err[0].startswith('* Dumped revision'):
> -    raise svntest.Failure('No valid output')
> +  if err:
> +    raise SVNUnexpectedStderr(err)
>  
>    # Compare the output from stdout
>    svntest.verify.compare_and_display_lines(

+1, thanks.

Re: svn commit: r978841 - in /subversion/trunk/subversion/tests/cmdline: svnrdump_tests.py svnrdump_tests_data/ svnrdump_tests_data/revision0.dump

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Ramkumar Ramachandra wrote on Sat, Jul 24, 2010 at 16:58:34 +0530:
> Hi Daniel,
> 
> Daniel Shahaf writes:
> > Please don't add "Review by" before I've actually reviewed the patch, thanks :-)
> 
> Um, right. I guess I misunderstood you when you asked me to put the
> 'Review by' in your previous email. I'll include it while actually
> committing, but not in the email.
> 

It's not a "in email" versus "in the commit", but rather "before I reviewed"
and "after I reviewed".

I ask you not to write "Review by: danielsh" until *after* I have reviewed the
diff (not just the idea; the actual diff).  Whether it's in email or in the
physical log message is immaterial

Clearer?

> > Perhaps you can use one of the UnexpectedStderr classes?  This has the
> > advantage that the unexpected stderr output would be printed to whomever is
> > running the test.
> 
> Excellent suggestion! Now fixed.
> 
> [[[
> * subversino/tests/cmdline/svnrdump_tests.py (run_test): Run svnrdump
>   with '-q' and check that nothing is printed to stderr.
> ]]]
> 
> Index: subversion/tests/cmdline/svnrdump_tests.py
> ===================================================================
> --- subversion/tests/cmdline/svnrdump_tests.py	(revision 978841)
> +++ subversion/tests/cmdline/svnrdump_tests.py	(working copy)
> @@ -73,15 +73,15 @@ def run_test(sbox, dumpfile_name):
>    svntest.actions.run_and_verify_load(sbox.repo_dir, svnadmin_dumpfile)
>  
>    # Create a dump file using svnrdump
> -  r, svnrdump_dumpfile, err = svntest.main.run_svnrdump(sbox.repo_url)
> +  r, svnrdump_dumpfile, err = svntest.main.run_svnrdump('-q', sbox.repo_url)
>  
>    # Check error code
>    if (r != 0):
>      raise svntest.Failure('Result code not 0')
>  
>    # Check the output from stderr
> -  if not err[0].startswith('* Dumped revision'):
> -    raise svntest.Failure('No valid output')
> +  if err:
> +    raise SVNUnexpectedStderr(err)
>  
>    # Compare the output from stdout
>    svntest.verify.compare_and_display_lines(

+1, thanks.

Re: svn commit: r978841 - in /subversion/trunk/subversion/tests/cmdline: svnrdump_tests.py svnrdump_tests_data/ svnrdump_tests_data/revision0.dump

Posted by Ramkumar Ramachandra <ar...@gmail.com>.
Hi Daniel,

Daniel Shahaf writes:
> Please don't add "Review by" before I've actually reviewed the patch, thanks :-)

Um, right. I guess I misunderstood you when you asked me to put the
'Review by' in your previous email. I'll include it while actually
committing, but not in the email.

> Perhaps you can use one of the UnexpectedStderr classes?  This has the
> advantage that the unexpected stderr output would be printed to whomever is
> running the test.

Excellent suggestion! Now fixed.

[[[
* subversino/tests/cmdline/svnrdump_tests.py (run_test): Run svnrdump
  with '-q' and check that nothing is printed to stderr.
]]]

Index: subversion/tests/cmdline/svnrdump_tests.py
===================================================================
--- subversion/tests/cmdline/svnrdump_tests.py	(revision 978841)
+++ subversion/tests/cmdline/svnrdump_tests.py	(working copy)
@@ -73,15 +73,15 @@ def run_test(sbox, dumpfile_name):
   svntest.actions.run_and_verify_load(sbox.repo_dir, svnadmin_dumpfile)
 
   # Create a dump file using svnrdump
-  r, svnrdump_dumpfile, err = svntest.main.run_svnrdump(sbox.repo_url)
+  r, svnrdump_dumpfile, err = svntest.main.run_svnrdump('-q', sbox.repo_url)
 
   # Check error code
   if (r != 0):
     raise svntest.Failure('Result code not 0')
 
   # Check the output from stderr
-  if not err[0].startswith('* Dumped revision'):
-    raise svntest.Failure('No valid output')
+  if err:
+    raise SVNUnexpectedStderr(err)
 
   # Compare the output from stdout
   svntest.verify.compare_and_display_lines(

Re: svn commit: r978841 - in /subversion/trunk/subversion/tests/cmdline: svnrdump_tests.py svnrdump_tests_data/ svnrdump_tests_data/revision0.dump

Posted by Ramkumar Ramachandra <ar...@gmail.com>.
Hi Daniel,

Daniel Shahaf writes:
> Please don't add "Review by" before I've actually reviewed the patch, thanks :-)

Um, right. I guess I misunderstood you when you asked me to put the
'Review by' in your previous email. I'll include it while actually
committing, but not in the email.

> Perhaps you can use one of the UnexpectedStderr classes?  This has the
> advantage that the unexpected stderr output would be printed to whomever is
> running the test.

Excellent suggestion! Now fixed.

[[[
* subversino/tests/cmdline/svnrdump_tests.py (run_test): Run svnrdump
  with '-q' and check that nothing is printed to stderr.
]]]

Index: subversion/tests/cmdline/svnrdump_tests.py
===================================================================
--- subversion/tests/cmdline/svnrdump_tests.py	(revision 978841)
+++ subversion/tests/cmdline/svnrdump_tests.py	(working copy)
@@ -73,15 +73,15 @@ def run_test(sbox, dumpfile_name):
   svntest.actions.run_and_verify_load(sbox.repo_dir, svnadmin_dumpfile)
 
   # Create a dump file using svnrdump
-  r, svnrdump_dumpfile, err = svntest.main.run_svnrdump(sbox.repo_url)
+  r, svnrdump_dumpfile, err = svntest.main.run_svnrdump('-q', sbox.repo_url)
 
   # Check error code
   if (r != 0):
     raise svntest.Failure('Result code not 0')
 
   # Check the output from stderr
-  if not err[0].startswith('* Dumped revision'):
-    raise svntest.Failure('No valid output')
+  if err:
+    raise SVNUnexpectedStderr(err)
 
   # Compare the output from stdout
   svntest.verify.compare_and_display_lines(

Re: svn commit: r978841 - in /subversion/trunk/subversion/tests/cmdline: svnrdump_tests.py svnrdump_tests_data/ svnrdump_tests_data/revision0.dump

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Ramkumar Ramachandra wrote on Sat, Jul 24, 2010 at 16:28:49 +0530:
> Hi Daniel,
> 
> Daniel Shahaf writes:
> > Not your fault, but that's not what I meant.  What I meant was to check for no
> > unexpected stderr (e.g., no "svn: warning %s" or similar).
> > 
> > For example, you could do that by running 'svnrdump -q' and then verifying that
> > *nothing* was printed to stderr.
> 
> Is this alright?
> 
> [[[
> * cmdline/svnrdump_tests.py (run_test): Run svnrdump with '-q' and

Please give the path relative to the root of trunk:

* subversion/tests/cmdline/svnrdump_tests.py
  (run_dump): %s

>   check that nothing is printed to stderr.
> 
> Review by: danielsh

Please don't add "Review by" before I've actually reviewed the patch, thanks :-)

(i.e., you can add it, if necessary, after I actually reviewed the first
iteration on-list)

> ]]]
> 
> Index: subversion/tests/cmdline/svnrdump_tests.py
> ===================================================================
> --- subversion/tests/cmdline/svnrdump_tests.py	(revision 978841)
> +++ subversion/tests/cmdline/svnrdump_tests.py	(working copy)
> @@ -73,15 +73,15 @@ def run_test(sbox, dumpfile_name):
>    svntest.actions.run_and_verify_load(sbox.repo_dir, svnadmin_dumpfile)
>  
>    # Create a dump file using svnrdump
> -  r, svnrdump_dumpfile, err = svntest.main.run_svnrdump(sbox.repo_url)
> +  r, svnrdump_dumpfile, err = svntest.main.run_svnrdump('-q', sbox.repo_url)
>  
>    # Check error code
>    if (r != 0):
>      raise svntest.Failure('Result code not 0')
>  
>    # Check the output from stderr
> -  if not err[0].startswith('* Dumped revision'):
> -    raise svntest.Failure('No valid output')
> +  if err:
> +    raise svntest.Failure('Error while running')
>  

Perhaps you can use one of the UnexpectedStderr classes?  This has the
advantage that the unexpected stderr output would be printed to whomever is
running the test.

(you can test this by adding an SVN_ERR_MALFUNCTION() in svnrdump.c:main())

>    # Compare the output from stdout
>    svntest.verify.compare_and_display_lines(

Re: svn commit: r978841 - in /subversion/trunk/subversion/tests/cmdline: svnrdump_tests.py svnrdump_tests_data/ svnrdump_tests_data/revision0.dump

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Ramkumar Ramachandra wrote on Sat, Jul 24, 2010 at 16:28:49 +0530:
> Hi Daniel,
> 
> Daniel Shahaf writes:
> > Not your fault, but that's not what I meant.  What I meant was to check for no
> > unexpected stderr (e.g., no "svn: warning %s" or similar).
> > 
> > For example, you could do that by running 'svnrdump -q' and then verifying that
> > *nothing* was printed to stderr.
> 
> Is this alright?
> 
> [[[
> * cmdline/svnrdump_tests.py (run_test): Run svnrdump with '-q' and

Please give the path relative to the root of trunk:

* subversion/tests/cmdline/svnrdump_tests.py
  (run_dump): %s

>   check that nothing is printed to stderr.
> 
> Review by: danielsh

Please don't add "Review by" before I've actually reviewed the patch, thanks :-)

(i.e., you can add it, if necessary, after I actually reviewed the first
iteration on-list)

> ]]]
> 
> Index: subversion/tests/cmdline/svnrdump_tests.py
> ===================================================================
> --- subversion/tests/cmdline/svnrdump_tests.py	(revision 978841)
> +++ subversion/tests/cmdline/svnrdump_tests.py	(working copy)
> @@ -73,15 +73,15 @@ def run_test(sbox, dumpfile_name):
>    svntest.actions.run_and_verify_load(sbox.repo_dir, svnadmin_dumpfile)
>  
>    # Create a dump file using svnrdump
> -  r, svnrdump_dumpfile, err = svntest.main.run_svnrdump(sbox.repo_url)
> +  r, svnrdump_dumpfile, err = svntest.main.run_svnrdump('-q', sbox.repo_url)
>  
>    # Check error code
>    if (r != 0):
>      raise svntest.Failure('Result code not 0')
>  
>    # Check the output from stderr
> -  if not err[0].startswith('* Dumped revision'):
> -    raise svntest.Failure('No valid output')
> +  if err:
> +    raise svntest.Failure('Error while running')
>  

Perhaps you can use one of the UnexpectedStderr classes?  This has the
advantage that the unexpected stderr output would be printed to whomever is
running the test.

(you can test this by adding an SVN_ERR_MALFUNCTION() in svnrdump.c:main())

>    # Compare the output from stdout
>    svntest.verify.compare_and_display_lines(

Re: svn commit: r978841 - in /subversion/trunk/subversion/tests/cmdline: svnrdump_tests.py svnrdump_tests_data/ svnrdump_tests_data/revision0.dump

Posted by Ramkumar Ramachandra <ar...@gmail.com>.
Hi Daniel,

Daniel Shahaf writes:
> Not your fault, but that's not what I meant.  What I meant was to check for no
> unexpected stderr (e.g., no "svn: warning %s" or similar).
> 
> For example, you could do that by running 'svnrdump -q' and then verifying that
> *nothing* was printed to stderr.

Is this alright?

[[[
* cmdline/svnrdump_tests.py (run_test): Run svnrdump with '-q' and
  check that nothing is printed to stderr.

Review by: danielsh
]]]

Index: subversion/tests/cmdline/svnrdump_tests.py
===================================================================
--- subversion/tests/cmdline/svnrdump_tests.py	(revision 978841)
+++ subversion/tests/cmdline/svnrdump_tests.py	(working copy)
@@ -73,15 +73,15 @@ def run_test(sbox, dumpfile_name):
   svntest.actions.run_and_verify_load(sbox.repo_dir, svnadmin_dumpfile)
 
   # Create a dump file using svnrdump
-  r, svnrdump_dumpfile, err = svntest.main.run_svnrdump(sbox.repo_url)
+  r, svnrdump_dumpfile, err = svntest.main.run_svnrdump('-q', sbox.repo_url)
 
   # Check error code
   if (r != 0):
     raise svntest.Failure('Result code not 0')
 
   # Check the output from stderr
-  if not err[0].startswith('* Dumped revision'):
-    raise svntest.Failure('No valid output')
+  if err:
+    raise svntest.Failure('Error while running')
 
   # Compare the output from stdout
   svntest.verify.compare_and_display_lines(

Re: svn commit: r978841 - in /subversion/trunk/subversion/tests/cmdline: svnrdump_tests.py svnrdump_tests_data/ svnrdump_tests_data/revision0.dump

Posted by Ramkumar Ramachandra <ar...@gmail.com>.
Hi Daniel,

Daniel Shahaf writes:
> Not your fault, but that's not what I meant.  What I meant was to check for no
> unexpected stderr (e.g., no "svn: warning %s" or similar).
> 
> For example, you could do that by running 'svnrdump -q' and then verifying that
> *nothing* was printed to stderr.

Is this alright?

[[[
* cmdline/svnrdump_tests.py (run_test): Run svnrdump with '-q' and
  check that nothing is printed to stderr.

Review by: danielsh
]]]

Index: subversion/tests/cmdline/svnrdump_tests.py
===================================================================
--- subversion/tests/cmdline/svnrdump_tests.py	(revision 978841)
+++ subversion/tests/cmdline/svnrdump_tests.py	(working copy)
@@ -73,15 +73,15 @@ def run_test(sbox, dumpfile_name):
   svntest.actions.run_and_verify_load(sbox.repo_dir, svnadmin_dumpfile)
 
   # Create a dump file using svnrdump
-  r, svnrdump_dumpfile, err = svntest.main.run_svnrdump(sbox.repo_url)
+  r, svnrdump_dumpfile, err = svntest.main.run_svnrdump('-q', sbox.repo_url)
 
   # Check error code
   if (r != 0):
     raise svntest.Failure('Result code not 0')
 
   # Check the output from stderr
-  if not err[0].startswith('* Dumped revision'):
-    raise svntest.Failure('No valid output')
+  if err:
+    raise svntest.Failure('Error while running')
 
   # Compare the output from stdout
   svntest.verify.compare_and_display_lines(