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 Sahlberg <da...@gmail.com> on 2022/01/27 20:14:17 UTC

Testsuite: svntest.actions.run_and_verify_log_xml with expected_revprops

Hi,

I'm working on issues #4711 and #4856 which involves merges and svn log
--use-merge-history --xml. I'm currently extending the tests
(log_with_merge_history_and_search and log_xml_with_merge_history).

I'm creating the branches using sandbox.simple_repo_copy. To verify the
output I'm using svntest.actions.run_and_verify_log_xml with
expected_revprops.

I'm running into problems since sandbox.simple_repo_copy add a log message
containing the path of the test script as well the line number.

I could work around it by implementing my own version of simple_repo_copy
but instead I'd like to propose to add an optional parameter for the log
message, as follows.

[[[
daniel@DESKTOP-DT42993:~/svn_4711/subversion/tests/cmdline$ svn diff
svntest/sandbox.py
Index: svntest/sandbox.py
===================================================================
--- svntest/sandbox.py  (revision 1897553)
+++ svntest/sandbox.py  (working copy)
@@ -450,11 +450,13 @@
     dest = self.ospath(dest)
     svntest.main.run_svn(False, 'move', source, dest)

-  def simple_repo_copy(self, source, dest):
+  def simple_repo_copy(self, source, dest, logmsg=''):
     """Copy SOURCE to DEST in the repository, committing the result with a
        default log message.
        SOURCE and DEST are relpaths relative to the repo root."""
-    svntest.main.run_svn(False, 'copy', '-m', svntest.main.make_log_msg(),
+    if not logmsg:
+      logmsg = svntest.main.make_log_msg()
+    svntest.main.run_svn(False, 'copy', '-m', logmsg,
                          self.repo_url + '/' + source,
                          self.repo_url + '/' + dest)
]]]

I'd like to run it by dev@ since I'm not that familiar with Python.

Kind regards,
Daniel Sahlberg

Re: Testsuite: svntest.actions.run_and_verify_log_xml with expected_revprops

Posted by Julian Foad <ju...@foad.me.uk>.
That looks like it would work and the proposal sounds ok. Comments:
- Is there precedent for an optional log message in nearby methods? If so, make it consistent.
- I would use 'None' as the argument default value, and change the test to 'if logmsg is None:' (literally), so the caller is able to set an empty message if they want to;
- As an alternative possible approach, did you see if the xml output matching function you are using can accept regular expression matching? Many of our matching functions in the test suite can, but I didn't look at this.

Anyway I'd support a change like this even if regex matching the output is an alternative.

- Julian
- Julian