Rev 4406: Get rid of _verbose_warning and rework the messages overall display (revealing some incoherences) and fixing the tests accordingly. in file:///home/vila/src/bzr/reviews/smooth-upgrades/

Vincent Ladeuil v.ladeuil+lp at free.fr
Fri Dec 17 11:35:29 GMT 2010


At file:///home/vila/src/bzr/reviews/smooth-upgrades/

------------------------------------------------------------
revno: 4406
revision-id: v.ladeuil+lp at free.fr-20101217113528-ixl4lx8pmnaartzt
parent: v.ladeuil+lp at free.fr-20101217101648-qilaa7n4gmhrd2f1
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: smooth-upgrades
timestamp: Fri 2010-12-17 12:35:28 +0100
message:
  Get rid of _verbose_warning and rework the messages overall display (revealing some incoherences) and fixing the tests accordingly.
-------------- next part --------------
=== modified file 'bzrlib/tests/blackbox/test_upgrade.py'
--- a/bzrlib/tests/blackbox/test_upgrade.py	2010-12-07 10:03:47 +0000
+++ b/bzrlib/tests/blackbox/test_upgrade.py	2010-12-17 11:35:28 +0000
@@ -58,32 +58,39 @@
         self.make_format_5_branch()
         (out, err) = self.run_bzr(
             ['upgrade', self.get_readonly_url('format_5_branch')], retcode=3)
-        self.assertEqual(out, "")
-        self.assertEqual(err, "bzr: ERROR: Upgrade URL cannot work with readonly URLs.\n")
+        err_msg = 'Upgrade URL cannot work with readonly URLs.'
+        self.assertEqualDiff('conversion error: %s\nbzr: ERROR: %s\n'
+                             % (err_msg, err_msg),
+                             err)
 
     def test_upgrade_up_to_date(self):
         self.make_current_format_branch_and_checkout()
         # when up to date we should get a message to that effect
         (out, err) = self.run_bzr('upgrade current_format_branch', retcode=3)
-        self.assertEqual("", out)
-        self.assertEqualDiff("bzr: ERROR: The branch format Meta "
-                             "directory format 1 is already at the most "
-                             "recent format.\n", err)
+        err_msg = ('The branch format %s is already at the most recent format.'
+                   % ('Meta directory format 1'))
+        self.assertEqualDiff('conversion error: %s\nbzr: ERROR: %s\n'
+                             % (err_msg, err_msg),
+                             err)
 
     def test_upgrade_up_to_date_checkout_warns_branch_left_alone(self):
         self.make_current_format_branch_and_checkout()
         # when upgrading a checkout, the branch location and a suggestion
         # to upgrade it should be emitted even if the checkout is up to
         # date
+        burl = self.get_transport('current_format_branch').base
+        curl = self.get_transport('current_format_checkout').base
         (out, err) = self.run_bzr('upgrade current_format_checkout', retcode=3)
-        self.assertEqual("This is a checkout. The branch (%s) needs to be "
-                         "upgraded separately.\n"
-                         % transport.get_transport(
-                self.get_url('current_format_branch')).base,
-                         out)
-        self.assertEqualDiff("bzr: ERROR: The branch format Meta "
-                             "directory format 1 is already at the most "
-                             "recent format.\n", err)
+        self.assertEqual(
+            'Upgrading branch %s ...\nThis is a checkout.'
+            ' The branch (%s) needs to be upgraded separately.\n'
+            % (curl, burl),
+            out)
+        msg = 'The branch format %s is already at the most recent format.' % (
+            'Meta directory format 1')
+        self.assertEqualDiff('conversion error: %s\nbzr: ERROR: %s\n'
+                             % (msg, msg),
+                             err)
 
     def test_upgrade_checkout(self):
         # upgrading a checkout should work
@@ -107,7 +114,8 @@
         backup_dir = 'backup.bzr.~1~'
         (out, err) = self.run_bzr(
             ['upgrade', '--format=metaweave', url])
-        self.assertEqualDiff("""starting upgrade of %s
+        self.assertEqualDiff("""Upgrading branch %s ...
+starting upgrade of %s
 making backup of %s.bzr
   to %s%s
 starting upgrade from format 5 to 6
@@ -115,7 +123,7 @@
 adding prefixes to revision-store
 starting upgrade from format 6 to metadir
 finished
-""" % (url, url, url, backup_dir), out)
+""" % (url, url, url, url, backup_dir), out)
         self.assertEqualDiff("", err)
         self.assertTrue(isinstance(
             bzrdir.BzrDir.open(self.get_url('format_5_branch'))._format,
@@ -131,13 +139,15 @@
         backup_dir = 'backup.bzr.~1~'
         (out, err) = self.run_bzr(
             ['upgrade', '--format=knit', url])
-        self.assertEqualDiff("""starting upgrade of %s
+        self.assertEqualDiff("""Upgrading branch %s ...
+starting upgrade of %s
 making backup of %s.bzr
   to %s%s
 starting repository conversion
 repository converted
 finished
-""" % (url, url, url, backup_dir), out)
+""" % (url, url, url, url, backup_dir),
+                             out)
         self.assertEqualDiff("", err)
         converted_dir = bzrdir.BzrDir.open(self.get_url('metadir_weave_branch'))
         self.assertTrue(isinstance(converted_dir._format,
@@ -196,7 +206,8 @@
         t.mkdir(backup_dir1)
         (out, err) = self.run_bzr(
             ['upgrade', '--format=metaweave', url])
-        self.assertEqualDiff("""starting upgrade of %s
+        self.assertEqualDiff("""Upgrading branch %s ...
+starting upgrade of %s
 making backup of %s.bzr
   to %s%s
 starting upgrade from format 5 to 6
@@ -204,7 +215,7 @@
 adding prefixes to revision-store
 starting upgrade from format 6 to metadir
 finished
-""" % (url, url, url, backup_dir2), out)
+""" % (url, url, url, url, backup_dir2), out)
         self.assertEqualDiff("", err)
         self.assertTrue(isinstance(
             bzrdir.BzrDir.open(self.get_url('format_5_branch'))._format,
@@ -220,14 +231,15 @@
         url = t.base
         out, err = self.run_bzr(['upgrade', '--format=knit', url])
         backup_dir = 'backup.bzr.~1~'
-        self.assertEqualDiff("""starting upgrade of %s
+        self.assertEqualDiff("""Upgrading branch %s ...
+starting upgrade of %s
 making backup of %s.bzr
   to %s%s
 starting upgrade from format 6 to metadir
 starting repository conversion
 repository converted
 finished
-""" % (url, url, url,backup_dir), out)
+""" % (url, url, url, url,backup_dir), out)
         self.assertEqual('', err)
 
 

=== modified file 'bzrlib/upgrade.py'
--- a/bzrlib/upgrade.py	2010-12-07 10:26:09 +0000
+++ b/bzrlib/upgrade.py	2010-12-17 11:35:28 +0000
@@ -21,11 +21,11 @@
     errors,
     osutils,
     repository,
+    trace,
     ui,
     )
 from bzrlib.bzrdir import BzrDir, format_registry
 from bzrlib.remote import RemoteBzrDir
-from bzrlib.trace import mutter, note, warning
 
 
 class Convert(object):
@@ -69,9 +69,9 @@
         try:
             branch = self.bzrdir.open_branch()
             if branch.user_url != self.bzrdir.user_url:
-                ui.ui_factory.note("This is a checkout. The branch (%s) needs to be "
-                             "upgraded separately." %
-                             branch.user_url)
+                ui.ui_factory.note(
+                    'This is a checkout. The branch (%s) needs to be upgraded'
+                    ' separately.' % (branch.user_url,))
             del branch
         except (errors.NotBranchError, errors.IncompatibleRepositories):
             # might not be a format we can open without upgrading; see e.g.
@@ -101,7 +101,7 @@
         while self.bzrdir.needs_format_conversion(format):
             converter = self.bzrdir._format.get_converter(format)
             self.bzrdir = converter.convert(self.bzrdir, None)
-        ui.ui_factory.note("finished")
+        ui.ui_factory.note('finished')
 
     def clean_up(self):
         """Clean-up after a conversion.
@@ -141,8 +141,9 @@
         attempted_count = len(attempted)
         succeeded_count = len(succeeded)
         failed_count = attempted_count - succeeded_count
-        note("\nSUMMARY: %d upgrades attempted, %d succeeded, %d failed",
-            attempted_count, succeeded_count, failed_count)
+        ui.ui_factory.note(
+            '\nSUMMARY: %d upgrades attempted, %d succeeded, %d failed'
+            % (attempted_count, succeeded_count, failed_count))
     return exceptions
 
 
@@ -195,10 +196,10 @@
     # Do the conversions
     attempted = [control_dir]
     succeeded, exceptions = _convert_items([control_dir], format, clean_up,
-        dry_run, verbose=dependents)
+                                           dry_run)
     if succeeded and dependents:
-        note("Found %d dependent branches - upgrading ...", len(dependents))
-
+        ui.ui_factory.note('Found %d dependent branches - upgrading ...'
+                           % (len(dependents),))
         # Convert dependent branches
         branch_cdirs = [b.bzrdir for b in dependents]
         successes, problems = _convert_items(branch_cdirs, format, clean_up,
@@ -211,8 +212,7 @@
     return attempted, succeeded, exceptions
 
 
-def _convert_items(items, format, clean_up, dry_run, label=None,
-    verbose=True):
+def _convert_items(items, format, clean_up, dry_run, label=None):
     """Convert a sequence of control directories to the given format.
  
     :param items: the control directories to upgrade
@@ -221,29 +221,24 @@
       upgrade succeeded for a given repo/branch/tree
     :param dry_run: show what would happen but don't actually do any upgrades
     :param label: the label for these items or None to calculate one
-    :param verbose: if True, output a message before starting and
-      display any problems encountered
     :return: items successfully upgraded, exceptions
     """
     succeeded = []
     exceptions = []
     child_pb = ui.ui_factory.nested_progress_bar()
-    i = 0
-    child_pb.update('Upgrading bzrdirs', i, len(items))
-    for control_dir in items:
-        i += 1
+    child_pb.update('Upgrading bzrdirs', 0, len(items))
+    for i, control_dir in enumerate(items):
         # Do the conversion
         location = control_dir.root_transport.base
         bzr_object, bzr_label = control_dir._get_object_and_label()
         type_label = label or bzr_label
-        child_pb.update("Upgrading %s" % (type_label), i, len(items))
-        if verbose:
-            note("Upgrading %s %s ...", type_label, location)
+        child_pb.update("Upgrading %s" % (type_label), i+1, len(items))
+        ui.ui_factory.note('Upgrading %s %s ...' % (type_label, location,))
         try:
             if not dry_run:
                 cv = Convert(control_dir=control_dir, format=format)
         except Exception, ex:
-            _verbose_warning(verbose, "conversion error: %s" % ex)
+            trace.warning('conversion error: %s' % ex)
             exceptions.append(ex)
             continue
 
@@ -251,21 +246,14 @@
         succeeded.append(control_dir)
         if clean_up:
             try:
-                note("Removing backup ...")
+                ui.ui_factory.note('Removing backup ...')
                 if not dry_run:
                     cv.clean_up()
             except Exception, ex:
-                _verbose_warning(verbose, "failed to clean-up %s: %s" %
-                    (location, ex))
+                trace.warning('failed to clean-up %s: %s' % (location, ex))
                 exceptions.append(ex)
 
     child_pb.finished()
 
     # Return the result
     return succeeded, exceptions
-
-
-def _verbose_warning(verbose, msg):
-    mutter(msg)
-    if verbose:
-        warning(msg)



More information about the bazaar-commits mailing list