Thanks Brock for quick response, more line ....

On 04/17/12 02:58, Brock Pytlik wrote:
t_pkg_publisher:
Line 454: Don't use the -O option. it's deprecated.

I have modifies the test to use '-g' (instead of '-O')
for quick ref :
+        def test_bug_7141684(self):
+ """Test that pkg(1) client does not traceback if no publisher
+                configured and we try to get preferred publisher"""
+
+                self.image_create()
+                self.pkg("publisher -P", exit=0)
+
+                self.pkg("set-publisher -g %s test" % self.durl1)
+                self.pkg("install foo")
+                self.pkg("unset-publisher test")
+                self.pkg("publisher -P", exit=0)

BTW I do see use of '-O' with set-publisher at many test cases.
As I am at it, should I file another bug for that that clean up ?
i.e '-O' -- > '-g'


Other than that. lgtm.

Thanks, I have attached the patch.
If this looks good please if you can integrate this on my behalf.

Thanks again,
~Saurabh

Brock

On 04/16/12 04:27, Saurabh Vyas wrote:
On 04/14/12 02:25, Brock Pytlik wrote:
t_pkg_publisher:
line 436: s/do/does, and watch the 80 character line limit, "publisher"
looks like it's over to me.

Instead of creating an image with a publisher, I'd prefer you did the
following things:
1) create the image without a publisher (ie, self.image_create())
2) check that publisher -P exits 0
3) add a publisher, install a package from that publisher, unset that
publisher
4) check that publisher -P exits 0 and has the correct output (ie, none
in this case)

You could also add a check between 2 and 3 where you add and remove a
publisher (essentially what the test is doing today) and then check the
output of publisher -P.

Thanks Brock for the review & your inputs.
I have enhanced the test to scenarios mentioned by you.

Updated webrev :
https://cr.opensolaris.org/action/browse/pkg/saurabhv/CR-7141684-rev7/webrev/


Please if you can verify.

Test run (for complete test suite) is clean.

Thanks,
~Saurabh



--


Saurabh Vyas
Solaris Install Group,
Revenue Product Engineering (RPE), Systems

ORACLE India | Off Langford Road | Bangalore | 560025

|Bangalore |
Green Oracle <http://www.oracle.com/commitment> Oracle is committed to
developing practices and products that help protect the environment
# HG changeset patch
# User [email protected]
# Date 1334640749 -19800
# Node ID 5443d3193f9ee277555ce4b83341dcc0c06b6304
# Parent  bd1b1e5569841e5e9ccf34ba94bb22f1c891e6b5
7141684 'pkg publisher -P' traceback when NO publisher is configured

diff -r bd1b1e556984 -r 5443d3193f9e src/client.py
--- a/src/client.py     Fri Apr 06 15:45:43 2012 -0700
+++ b/src/client.py     Tue Apr 17 11:02:29 2012 +0530
@@ -21,7 +21,7 @@
 #
 
 #
-# Copyright (c) 2007, 2011, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2007, 2012, Oracle and/or its affiliates. All rights reserved.
 #
 
 #
@@ -4526,9 +4526,14 @@
 
         retcode = EXIT_OK
         if len(pargs) == 0:
-                pref_pub = api_inst.get_highest_ranked_publisher()
                 if preferred_only:
-                        pubs = [pref_pub]
+                        pref_pub = api_inst.get_highest_ranked_publisher()
+                        if api_inst.has_publisher(pref_pub):
+                                pubs = [pref_pub]
+                        else:
+                                # Only publisher known is from an installed
+                                # package and is not configured in the image.
+                                pubs = []
                 else:
                         pubs = [
                             p for p in api_inst.get_publishers()
@@ -4595,11 +4600,15 @@
 
                         # Only show the selected repository's information in
                         # summary view.
-                        r = p.repository
+                        if p.repository:
+                                origins = p.repository.origins
+                                mirrors = p.repository.mirrors
+                        else:
+                                origins = mirrors = [] 
 
                         # Update field_data for each origin and output
                         # a publisher record in our desired format.
-                        for uri in sorted(r.origins):
+                        for uri in sorted(origins):
                                 # XXX get the real origin status
                                 set_value(field_data["type"], _("origin"))
                                 set_value(field_data["status"], _("online"))
@@ -4611,7 +4620,7 @@
                                 msg(fmt % tuple(values))
                         # Update field_data for each mirror and output
                         # a publisher record in our desired format.
-                        for uri in r.mirrors:
+                        for uri in mirrors:
                                 # XXX get the real mirror status
                                 set_value(field_data["type"], _("mirror"))
                                 set_value(field_data["status"], _("online"))
@@ -4622,7 +4631,7 @@
                                 )
                                 msg(fmt % tuple(values))
 
-                        if not r.origins and not r.mirrors:
+                        if not origins and not mirrors:
                                 set_value(field_data["type"], "")
                                 set_value(field_data["status"], "")
                                 set_value(field_data["uri"], "")
diff -r bd1b1e556984 -r 5443d3193f9e src/tests/cli/t_pkg_publisher.py
--- a/src/tests/cli/t_pkg_publisher.py  Fri Apr 06 15:45:43 2012 -0700
+++ b/src/tests/cli/t_pkg_publisher.py  Tue Apr 17 11:02:29 2012 +0530
@@ -43,6 +43,18 @@
         # Tests in this suite use the read only data directory.
         need_ro_data = True
 
+        # Dummy package
+        foo1 = """
+            open foo@1,5.11-0
+            close """
+
+        def setUp(self):
+                # This test suite needs actual depots.
+                pkg5unittest.SingleDepotTestCase.setUp(self, start_depot=True)
+
+                self.durl1 = self.dcs[1].get_depot_url()
+                self.pkgsend_bulk(self.durl1, self.foo1)
+
         def test_pkg_publisher_bogus_opts(self):
                 """ pkg bogus option checks """
 
@@ -432,6 +444,17 @@
                 self.pkg("publisher test")
                 self.assert_("signature-required-names = n1, n2" in 
self.output)
 
+        def test_bug_7141684(self):
+                """Test that pkg(1) client does not traceback if no publisher
+                configured and we try to get preferred publisher"""
+
+                self.image_create()
+                self.pkg("publisher -P", exit=0)
+
+                self.pkg("set-publisher -g %s test" % self.durl1)
+                self.pkg("install foo")
+                self.pkg("unset-publisher test")
+                self.pkg("publisher -P", exit=0)
 
 class TestPkgPublisherMany(pkg5unittest.ManyDepotTestCase):
         # Only start/stop the depot once (instead of for every test)
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to