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