Hi Sue,
I incorporated your suggestions and re-run the test-suite.
New webrev is here:
https://cr.opensolaris.org/action/browse/caiman/t.dzik/7117539-1/
Could you please look at it ?
Thanks a lot in advance,
Tomas D.
Dne 5.12.11 17:44, Sue Sohn napsal(a):
On 12/ 5/11 08:40 AM, Tomas Dzik wrote:
Hi Sue, see in-line
Dne 5.12.11 17:05, Sue Sohn napsal(a):
On 12/ 5/11 04:59 AM, Tomas Dzik wrote:
Hi all,
I would like to ask you for a code review for the following bug:
7117539 installadm prints network criteria unformated
Webrev is here:
https://cr.opensolaris.org/action/browse/caiman/t.dzik/7117539/
Testing:
1) Sources are pep8 clean
2) I run the test-suite in gate (and fixed also the test suite).
3) I installed my fix on test machine and verified that network
is now printed formated as ipv4 address.
Output before fix:
root@S11:~# installadm list -m -n default-i386
Manifest Status Criteria
-------- ------ --------
default ipv4 = 192.168.1.15
network = 192168001000
orig_default Default None
Output after fix:
root@S11:~# installadm list -m -n default-i386
Manifest Status Criteria
-------- ------ --------
default arch = i386
ipv4 = 192.168.1.15
network = 192.168.1.0
orig_default Default None
Hi Tomas,
In test_ai_database.py, in addition to the typo pointed out by Darren,
249 The name of this test is test_arch_formatValue, but it is verifying
ipv4, not arch. Can you please correct the name? I know that you didn't
modify this, but since you are in there...
Fixed.
Also, since the tests at 249 and your new test at 255 both verify the
formatting of ipv4 addresses, shouldn't they be doing the verification
the same way?
I would say yes. Basically I didn't like the way how original test was
written (because it is in my
opinion too general) and wasn't brave enough to change it. So I have
additional questions:
a) Which test to use for both cases ? The original or test in
test_network_formatValue ?
Perhaps you can combine the tests together.
b) Should I keep these 2 test cases separated (I would prefer it) or
to combine them into 1 test case ?
I like having separate test cases too. Maybe you can create a method
they both use to do the verification.
Sue
Best regards,
Tomas D.
Sue
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
