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









--


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
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to