t_pkg_publisher:
Line 454: Don't use the -O option. it's deprecated.
Other than that. lgtm.
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
Brock
On 04/13/12 02:04, Saurabh Vyas wrote:
Hi Brock / Tim,
Please if you can spend some time reviewing this fix.
This is long pending, just wanted to take this to closure.
For quick ref :
CR 7141684 'pkg publisher -P' traceback when NO publisher is configured
webrev :
https://cr.opensolaris.org/action/browse/pkg/saurabhv/CR-7141684-rev5/webrev/
Thanks in advance,
~Saurabh
On 03/26/12 21:45, Saurabh Vyas wrote:
Hi All,
A gentle reminder, please if anyone (other than Shawn) can
look into this fix.
More details in mail below.
Thanks in advance,
Saurabh
On 03/22/12 12:11 AM, Saurabh Vyas wrote:
Thanks Shawn for quick response, more inline......
On 03/21/12 08:13 PM, Shawn Walker wrote:
On 03/21/12 04:24, Saurabh Vyas wrote:
...
...
I have made the changes as suggested, updated webrev :
https://cr.opensolaris.org/action/browse/pkg/saurabhv/CR-7141684-rev5/webrev/
src/tests/cli/t_pkg_publisher.py:
lines 435-441: This is indented one level too far, so it's currently
being treated as a nested function of test_publisher_properties.
De-indent this and re-run just this test to verify.
Thanks for catching this, I have corrected the indentation.
Re-ran the test, runs successfully.
Update webrev too :
https://cr.opensolaris.org/action/browse/pkg/saurabhv/CR-7141684-rev6/webrev/
Add a test case to src/tests/cli/t_pkg_publisher.py.
I have added test case for this bug, ran the test suite (runs
fine).
Just to be sure I just ran the test case I added on the tip of gate
without this fix, and got the traceback.
That's the correct way to test.
Thanks for confirming this.....
Please let me know your thoughts / comments ......
Since I helped you write this, I can't really be your final
reviewer.
Someone else will need to review this.
Please if some one else (apart from Shawn) can provide their review
comments for this fix.
Thanks again,
Saurabh
-Shawn
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
